[CHERI-RISC-V] Add a target feature to no longer assume ISAv8 semantics #708

arichardson · 2023-07-06T15:36:57Z

When set, this feature allows the compiler to assume new capability
semantics (tag-clearing instead of trapping and removal of DDC/PCC
relocation). This will allow hoisting previously trapping instructions
out of loops which cannot be done unconditionally now as we still want
to support building and running the last CheriBSD release with the latest
dev branch of the compiler. We therefore need an opt-in flag that can be
set by the CheriBSD build system.

This PR makes use of this target feature to assume non-trapping semantics for capability modification instructions which results in minor changes to codegen for Spec2k6.

Program        machinelicm.NumHoisted                 machine-cse.NumCSEs                size..text             
               baseline               new      diff   baseline            new     diff   baseline   new     diff
 483.xalancbmk  8002.00                8021.00  19.00 9620.00             9621.00   1.00 3212340    3215020 2680
     458.sjeng  2015.00                2022.00   7.00 2217.00             2214.00  -3.00  575424     575368  -56
     473.astar   238.00                 244.00   6.00  153.00              149.00  -4.00  570512     570552   40
   471.omnetpp  1769.00                1771.00   2.00 1830.00             1830.00   0.00 1314912    1315272  360
     401.bzip2   890.00                 891.00   1.00  517.00              517.00   0.00  498216     498176  -40
     456.hmmer  3876.00                3877.00   1.00 2099.00             2102.00   3.00  717652     717644   -8
     445.gobmk 12195.00               12195.00   0.00 6967.00             6967.00   0.00 1149532    1149492  -40
462.libquantum   164.00                 164.00   0.00  137.00              137.00   0.00  474088     474048  -40
   464.h264ref  6091.00                6091.00   0.00 7431.00             7431.00   0.00 1081220    1081180  -40

llvm/lib/Target/RISCV/RISCV.td

bsdjhb

In general looks fine to me, but I don't know the LLVM bits well enough to give a qualified review.

clang/test/Preprocessor/cheri-riscv-feature-flags.c

arichardson · 2023-08-07T14:45:08Z

@jrtc27 ping? Would be great to get this merged soon.

llvm/test/CodeGen/RISCV/cheri/machinelicm-hoist.mir

jrtc27 · 2023-08-08T17:54:57Z

llvm/lib/Target/RISCV/RISCV.td

+def FeatureNoCheriV8Compat
+    : SubtargetFeature<"xcheri-no-v8-compat", "HasCheriISAv8Semantics", "false",
+                       "CHERI ISAv8 semantics (trapping, DDC/PCC relocation)">;


The TableGen, subtarget feature and Subtarget variable names are all pairwise inconsistent

I originally started with a positive V9 semantics flag, but that did not out for some reason I've now forgotten since it's been a while. I think it caused problems when I tried to introduce the xcherimin feature.

Should be better now?

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

llvm/test/CodeGen/RISCV/cheri/isav9-cap-to-ptr.ll

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

llvm/test/CodeGen/RISCV/cheri/isav9-cap-from-ptr.ll

clang/test/Preprocessor/cheri-riscv-feature-flags.c

jrtc27 · 2023-08-08T18:10:05Z

clang/test/Preprocessor/cheri-riscv-feature-flags.c

This duplicates what's in init.c

This is required due to the --implicit-check-not=, I could restrict it to lowercase cheri, but I think it's helpful to have all macros that include CHERI to be checked here.

clang/test/Preprocessor/cheri-riscv-feature-flags.c

jrtc27

A few more minor points, ok to merge once fixed

llvm/lib/Target/RISCV/RISCV.td

llvm/lib/Target/RISCV/RISCVInstrInfoXCheri.td

llvm/test/CodeGen/RISCV/cheri/isav9-ptrtoint.ll

When set, this feature allows the compiler to assume new capability semantics (tag-clearing instead of trapping and removal of DDC/PCC relocation). This will allow hoisting previously trapping instructions out of loops which cannot be done unconditionally now as we still want to support building and running the last CheriBSD release with the latest dev branch of the compiler. We therefore need an opt-in flag that can be set by the CheriBSD build system.

When +xcheri-v9-semantics is set, we can assume that all capability modification instruction no longer trap. This is currently done in the isGuaranteedNotToTrap() hook, but once ISAv8 semantics are no longer supported we should update the tablegen instruction definitions to remove the mayTrap flag. This allows hoisting a few more instructions in MachineLICM which results in minor code quality improvements (for example 19 new machinelicm.NumHoisted in SPEC2k6 483.xalancbmk)

CToPtr will be removed from the CHERI-RISC-V ISA in the future and will not be part of a standardized version. We emit a branch-less form of `x.tag ? x.addr : 0` when we see the intrinsic in ISAv9 compilation mode instead. When expanding `ptrtoint` in hybrid mode we also no longer subtract the base of DDC as part of the expansion, since DDC relocation is no longer part of xcherimin, and subtracting the base would result in an integer that points to a different address when dereferenced. This behaviour is consistent with Morello when CCTLR.DDCBO is disabled.

We emit `x == 0 ? null : CSetAddrt(auth, x)` when we see the intrinsic in non-ISAv8 compilation mode instead. We use CSetAddr instead of CSetOffset as adding the base would result in a capability that points to a different address when dereferenced. This change in intrinsic behaviour is consistent with Morello where the resulting CVT instruction takes the DDC relocation configuration (CCTLR.DDCBO) into account. It would be nice if we could just use `CSetAddr(DDC, x)` unconditionally for non-constant `inttoptt` arguments, but that could result in semantic differences (tagged vs untagged zero) for integers that happen to be zero at run time.

This will be extended to show new macros to detect missing ISAv8 compat. This duplicates some checks from init.c, but this is required to ensure exhaustive checking with --implicit-check-not

This defines two new macros, `__riscv_xcheri_no_relocation` and `__riscv_xcheri_tag_clear` when the target feature +xcheri-no-v8-compat is enabled.

This will be removed once we default to the new semantics. This also adds a mxcheri-v8-compat flag to opt-in to the old semantics (although that is currently the default).

arichardson requested a review from jrtc27 July 6, 2023 15:36

arichardson commented Jul 6, 2023

View reviewed changes

llvm/lib/Target/RISCV/RISCV.td Outdated Show resolved Hide resolved

arichardson force-pushed the cheri-v9-semantics branch 3 times, most recently from 0db0587 to 3852466 Compare July 7, 2023 00:26

bsdjhb reviewed Jul 7, 2023

View reviewed changes

llvm/lib/Target/RISCV/RISCV.td Outdated Show resolved Hide resolved

arichardson mentioned this pull request Jul 7, 2023

[CHERI-RISC-V] Restrict CToPtr and CFromPtr usage to ISAv8 compatibility #711

Closed

arichardson force-pushed the cheri-v9-semantics branch from a173037 to 1ac5ee9 Compare July 7, 2023 21:28

bsdjhb reviewed Jul 7, 2023

View reviewed changes

clang/test/Preprocessor/cheri-riscv-feature-flags.c Outdated Show resolved Hide resolved

arichardson force-pushed the cheri-v9-semantics branch from 1ac5ee9 to d9f4b09 Compare July 10, 2023 16:00

arichardson mentioned this pull request Jul 12, 2023

Remove C{To,From}Ptr CTSRD-CHERI/sail-cheri-riscv#73

Open

jrtc27 reviewed Aug 8, 2023

View reviewed changes

[CHERI-RISC-V] Add CGetHigh/CSetHigh to machinelicm-hoist test

8cc82ae

arichardson requested a review from jrtc27 August 8, 2023 22:22

arichardson force-pushed the cheri-v9-semantics branch 2 times, most recently from f7cd93e to 19a41d6 Compare August 9, 2023 01:25

jrtc27 approved these changes Aug 9, 2023

View reviewed changes

llvm/lib/Target/RISCV/RISCV.td Outdated Show resolved Hide resolved

llvm/lib/Target/RISCV/RISCVInstrInfoXCheri.td Outdated Show resolved Hide resolved

llvm/test/CodeGen/RISCV/cheri/isav9-ptrtoint.ll Outdated Show resolved Hide resolved

arichardson added 7 commits August 9, 2023 11:11

[CHERI-RISC-V] Add a test for RISC-V CHERI preprocessor macros

73da86d

This will be extended to show new macros to detect missing ISAv8 compat. This duplicates some checks from init.c, but this is required to ensure exhaustive checking with --implicit-check-not

[CHERI-RISC-V] Add preprocessor macros to detect new CHERI semantics

6812cd8

This defines two new macros, `__riscv_xcheri_no_relocation` and `__riscv_xcheri_tag_clear` when the target feature +xcheri-no-v8-compat is enabled.

[CHERI-RISC-V] Add a temporary -mxcheri-v9-semantics clang flag

7fe8d14

This will be removed once we default to the new semantics. This also adds a mxcheri-v8-compat flag to opt-in to the old semantics (although that is currently the default).

arichardson force-pushed the cheri-v9-semantics branch from 19a41d6 to 7fe8d14 Compare August 9, 2023 18:15

arichardson merged commit 3f3b302 into dev Aug 9, 2023
4 checks passed

arichardson deleted the cheri-v9-semantics branch August 9, 2023 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CHERI-RISC-V] Add a target feature to no longer assume ISAv8 semantics #708

[CHERI-RISC-V] Add a target feature to no longer assume ISAv8 semantics #708

arichardson commented Jul 6, 2023

bsdjhb left a comment

arichardson commented Aug 7, 2023

jrtc27 Aug 8, 2023

arichardson Aug 8, 2023

arichardson Aug 8, 2023

jrtc27 Aug 8, 2023

arichardson Aug 8, 2023

jrtc27 left a comment

[CHERI-RISC-V] Add a target feature to no longer assume ISAv8 semantics #708

[CHERI-RISC-V] Add a target feature to no longer assume ISAv8 semantics #708

Conversation

arichardson commented Jul 6, 2023

bsdjhb left a comment

Choose a reason for hiding this comment

arichardson commented Aug 7, 2023

jrtc27 Aug 8, 2023

Choose a reason for hiding this comment

arichardson Aug 8, 2023

Choose a reason for hiding this comment

arichardson Aug 8, 2023

Choose a reason for hiding this comment

jrtc27 Aug 8, 2023

Choose a reason for hiding this comment

arichardson Aug 8, 2023

Choose a reason for hiding this comment

jrtc27 left a comment

Choose a reason for hiding this comment