[CHERI] Fix issue where tag_get was being CSE'd across free calls.

CHERIoT-Platform · Dec 16, 2024 · a906bf4 · a906bf4
1 parent ee28636
commit a906bf4
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 25 deletions.
diff --git a/clang/test/CodeGen/cheri/cheri.c b/clang/test/CodeGen/cheri/cheri.c
@@ -68,7 +68,7 @@
 // CAPS: call i64 @llvm.cheri.cap.flags.get.i64(ptr addrspace(200)
 // CAPS: define dso_local ptr addrspace(200) @cheri_flags_set(ptr addrspace(200) noundef readnone{{( %.+)?}}, i16 noundef zeroext
 // CAPS: call ptr addrspace(200) @llvm.cheri.cap.flags.set.i64(ptr addrspace(200){{( %.+)?}}, i64
-// CAPS: define dso_local zeroext i1 @cheri_tag_get(ptr addrspace(200) noundef readnone
+// CAPS: define dso_local zeroext i1 @cheri_tag_get(ptr addrspace(200) noundef readonly
 // CAPS: call i1 @llvm.cheri.cap.tag.get(ptr addrspace(200)
 // CAPS: define dso_local zeroext i1 @cheri_sealed_get(ptr addrspace(200) noundef readnone
 // CAPS: call i1 @llvm.cheri.cap.sealed.get(ptr addrspace(200)

diff --git a/clang/test/CodeGen/cheri/cheriintrin.c b/clang/test/CodeGen/cheri/cheriintrin.c
@@ -1,9 +1,9 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %cheri_cc1 %s -o - -emit-llvm -O1 -Weverything -Werror -verify -Wno-declaration-after-statement | FileCheck %s
-// RUN: %cheri_purecap_cc1 %s -o - -emit-llvm -O1 -Weverything -Werror -Wno-declaration-after-statement -verify | FileCheck %s
+// RUN: %cheri_cc1 %s -o - -emit-llvm -I. -O1 -Weverything -Werror -verify -Wno-declaration-after-statement | FileCheck %s
+// RUN: %cheri_purecap_cc1 %s -o - -emit-llvm -I. -O1 -Weverything -Werror -Wno-declaration-after-statement -verify | FileCheck %s
 // expected-no-diagnostics
 
-#include <cheriintrin.h>
+#include "cheriintrin.h"
 // Check that all macros defined in cheriintrin.h work as expected
 
 void use_size_t(__SIZE_TYPE__ s);
@@ -30,8 +30,10 @@ void test(void *__capability cap, char *__capability cap2, __SIZE_TYPE__ i);
 // CHECK-NEXT:    tail call void @use_cap(ptr addrspace(200) noundef [[TMP6]]) #[[ATTR5]]
 // CHECK-NEXT:    [[TMP7:%.*]] = tail call i1 @llvm.cheri.cap.tag.get(ptr addrspace(200) [[CAP]])
 // CHECK-NEXT:    tail call void @use_bool(i1 noundef zeroext [[TMP7]]) #[[ATTR5]]
-// CHECK-NEXT:    tail call void @use_bool(i1 noundef zeroext [[TMP7]]) #[[ATTR5]]
-// CHECK-NEXT:    [[LNOT:%.*]] = xor i1 [[TMP7]], true
+// CHECK-NEXT:    [[TMP32:%.*]] = tail call i1 @llvm.cheri.cap.tag.get(ptr addrspace(200) [[CAP]])
+// CHECK-NEXT:    tail call void @use_bool(i1 noundef zeroext [[TMP32]]) #[[ATTR5]]
+// CHECK-NEXT:    [[TMP33:%.*]] = tail call i1 @llvm.cheri.cap.tag.get(ptr addrspace(200) [[CAP]])
+// CHECK-NEXT:    [[LNOT:%.*]] = xor i1 [[TMP33]], true
 // CHECK-NEXT:    tail call void @use_bool(i1 noundef zeroext [[LNOT]]) #[[ATTR5]]
 // CHECK-NEXT:    [[TMP8:%.*]] = tail call i1 @llvm.cheri.cap.equal.exact(ptr addrspace(200) [[CAP]], ptr addrspace(200) [[CAP2:%.*]])
 // CHECK-NEXT:    tail call void @use_bool(i1 noundef zeroext [[TMP8]]) #[[ATTR5]]

diff --git a/llvm/include/llvm/IR/IntrinsicsCHERICap.td b/llvm/include/llvm/IR/IntrinsicsCHERICap.td
@@ -65,7 +65,7 @@ def int_cheri_cap_tag_get  :
   ClangBuiltin<"__builtin_cheri_tag_get">,
   Intrinsic<[llvm_i1_ty],
             [llvm_cap_ty],
-            [IntrNoMem, IntrWillReturn]>;
+            [IntrReadMem, IntrWillReturn]>;
 def int_cheri_cap_sealed_get  :
   ClangBuiltin<"__builtin_cheri_sealed_get">,
   Intrinsic<[llvm_i1_ty],

diff --git a/llvm/lib/Target/Mips/MipsISelLowering.cpp b/llvm/lib/Target/Mips/MipsISelLowering.cpp
@@ -570,6 +570,7 @@ MipsTargetLowering::MipsTargetLowering(const MipsTargetMachine &TM,
   if (Subtarget.isCheri()) {
     // We also fold Incoffsets to andaddr if possible
     setTargetDAGCombine(ISD::INTRINSIC_WO_CHAIN);
+    setTargetDAGCombine(ISD::INTRINSIC_W_CHAIN);
     setTargetDAGCombine(ISD::PTRADD);
   }
 
@@ -1346,14 +1347,6 @@ static SDValue performINTRINSIC_WO_CHAINCombine(
   // Lower to our custom node, but with a truncate back to i1 so we can
   // replace its uses.
   switch (IID) {
-  case Intrinsic::cheri_cap_tag_get: {
-    SDValue IntRes = DAG.getNode(MipsISD::CapTagGet, DL, VT,
-                                 N->getOperand(1));
-    IntRes = DAG.getNode(ISD::AssertZext, DL, VT, IntRes,
-                         DAG.getValueType(MVT::i1));
-    return DAG.getSetCC(DL, MVT::i1, IntRes,
-                        DAG.getConstant(0, DL, VT), ISD::SETNE);
-  }
   case Intrinsic::cheri_cap_sealed_get: {
     SDValue IntRes = DAG.getNode(MipsISD::CapSealedGet, DL, VT,
                                  N->getOperand(1));
@@ -1484,6 +1477,22 @@ SDValue  MipsTargetLowering::PerformDAGCombine(SDNode *N, DAGCombinerInfo &DCI)
     return performCIncOffsetToCandAddrCombine(N, DAG, DCI, Subtarget);
   case ISD::INTRINSIC_WO_CHAIN:
     return performINTRINSIC_WO_CHAINCombine(N, DAG, DCI, Subtarget);
+  case ISD::INTRINSIC_W_CHAIN: {
+    unsigned IID = cast<ConstantSDNode>(N->getOperand(1))->getZExtValue();
+    if (IID == Intrinsic::cheri_cap_tag_get) {
+      SDLoc DL(N);
+      EVT VT = Subtarget.isGP64bit() ? MVT::i64 : MVT::i32;
+      SDValue IntRes =
+          DAG.getNode(MipsISD::CapTagGet, DL, VT, N->getOperand(2));
+      IntRes = DAG.getNode(ISD::AssertZext, DL, VT, IntRes,
+                           DAG.getValueType(MVT::i1));
+      IntRes = DAG.getSetCC(DL, MVT::i1, IntRes, DAG.getConstant(0, DL, VT),
+                            ISD::SETNE);
+      DCI.CombineTo(N, {IntRes, N->getOperand(0)});
+      return SDValue();
+    }
+    break;
+  }
   }
 
   return SDValue();

diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -1367,6 +1367,12 @@ void RISCVDAGToDAGISel::Select(SDNode *Node) {
       return;
     break;
   }
+  case RISCVISD::CAP_TAG_GET: {
+    ReplaceNode(Node, CurDAG->getMachineNode(
+                          RISCV::CGetTag, DL, Node->getVTList(),
+                          {Node->getOperand(0), Node->getOperand(1)}));
+    return;
+  }
   case ISD::INTRINSIC_WO_CHAIN: {
     unsigned IntNo = Node->getConstantOperandVal(0);
     switch (IntNo) {

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -4505,7 +4505,9 @@ static SDValue emitCToPtrReplacement(SelectionDAG &DAG, const SDLoc &DL,
   // Ideally we would keep the select here and allow follow-up folds to optimize
   // the select depending on available instructions, but the MVT::i1 intrinsic
   // call is optimized poorly, so we expand it manually.
-  SDValue IsTagged = DAG.getNode(RISCVISD::CAP_TAG_GET, DL, XLenVT, Cap);
+  SDVTList VTList = DAG.getVTList(XLenVT, MVT::Other);
+  SDValue IsTagged =
+      DAG.getNode(RISCVISD::CAP_TAG_GET, DL, VTList, Cap, DAG.getEntryNode());
   SDValue Mask = DAG.getNode(ISD::SUB, DL, XLenVT,
                              DAG.getConstant(0, DL, XLenVT), IsTagged);
   // Using EXTRACT_SUBREG instead of getaddr is safe here since the result is
@@ -13574,12 +13576,15 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
     // Lower to our custom node, but with a truncate back to i1 so we can
     // replace its uses.
     case Intrinsic::cheri_cap_tag_get: {
-      SDValue IntRes =
-          DAG.getNode(RISCVISD::CAP_TAG_GET, DL, XLenVT, N->getOperand(1));
+      SDVTList VTList = DAG.getVTList(XLenVT, MVT::Other);
+      SDValue IntRes = DAG.getNode(RISCVISD::CAP_TAG_GET, DL, VTList,
+                                   N->getOperand(2), N->getOperand(0));
+      SDValue Chain = SDValue(IntRes.getNode(), 1);
       IntRes = DAG.getNode(ISD::AssertZext, DL, XLenVT, IntRes,
                            DAG.getValueType(MVT::i1));
-      return DAG.getSetCC(DL, MVT::i1, IntRes, DAG.getConstant(0, DL, XLenVT),
-                          ISD::SETNE);
+      SDValue SetCC = DAG.getSetCC(DL, MVT::i1, IntRes,
+                                   DAG.getConstant(0, DL, XLenVT), ISD::SETNE);
+      return DCI.CombineTo(N, SetCC, Chain);
     }
     case Intrinsic::cheri_cap_sealed_get: {
       SDValue IntRes =

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXCheri.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXCheri.td
@@ -17,6 +17,10 @@ def SDT_RISCVCheriBoolUnary : SDTypeProfile<1, 1, [
   SDTCisInt<0>, SDTCisVT<1, CLenVT>
 ]>;
 
+def SDT_RISCVCheriBoolUnaryChain : SDTypeProfile<2, 2, [
+  SDTCisInt<0>, SDTCisVT<2, CLenVT>
+]>;
+
 def SDT_RISCVCheriBoolBinary : SDTypeProfile<1, 2, [
   SDTCisInt<0>, SDTCisVT<1, CLenVT>, SDTCisVT<2, CLenVT>
 ]>;
@@ -35,7 +39,8 @@ def riscv_cap_tail        : SDNode<"RISCVISD::CAP_TAIL", SDT_RISCVCapCall,
                                    [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
                                     SDNPVariadic]>;
 def riscv_cap_tag_get     : SDNode<"RISCVISD::CAP_TAG_GET",
-                                   SDT_RISCVCheriBoolUnary>;
+                                   SDT_RISCVCheriBoolUnaryChain,
+                                   [SDNPHasChain]>;
 def riscv_cap_sealed_get  : SDNode<"RISCVISD::CAP_SEALED_GET",
                                    SDT_RISCVCheriBoolUnary>;
 def riscv_cap_subset_test : SDNode<"RISCVISD::CAP_SUBSET_TEST",
@@ -370,7 +375,8 @@ def CGetPerm   : Cheri_r<0x0, "cgetperm">;
 def CGetType   : Cheri_r<0x1, "cgettype">;
 def CGetBase   : Cheri_r<0x2, "cgetbase">;
 def CGetLen    : Cheri_r<0x3, "cgetlen">;
-def CGetTag    : Cheri_r<0x4, "cgettag">;
+let mayLoad = true in
+  def CGetTag    : Cheri_r<0x4, "cgettag">;
 def CGetSealed : Cheri_r<0x5, "cgetsealed">;
 def CGetOffset : Cheri_r<0x6, "cgetoffset">;
 def CGetFlags  : Cheri_r<0x7, "cgetflags">;
@@ -1280,7 +1286,6 @@ def : PatGpcr<int_cheri_cap_perms_get, CGetPerm>;
 def : PatGpcr<int_cheri_cap_type_get, CGetType>;
 def : PatGpcr<int_cheri_cap_base_get, CGetBase>;
 def : PatGpcr<int_cheri_cap_length_get, CGetLen>;
-def : PatGpcr<riscv_cap_tag_get, CGetTag>;
 def : PatGpcr<riscv_cap_sealed_get, CGetSealed>;
 def : PatGpcr<int_cheri_cap_offset_get, CGetOffset>;
 def : PatGpcr<int_cheri_cap_flags_get, CGetFlags>;

diff --git a/llvm/test/CodeGen/RISCV/cheri/machinelicm-hoist.mir b/llvm/test/CodeGen/RISCV/cheri/machinelicm-hoist.mir
@@ -95,7 +95,6 @@ body:             |
   ; CHECK-NEXT:   [[CGetType:%[0-9]+]]:gpr = CGetType [[COPY2]]
   ; CHECK-NEXT:   [[CGetBase:%[0-9]+]]:gpr = CGetBase [[COPY2]]
   ; CHECK-NEXT:   [[CGetLen:%[0-9]+]]:gpr = CGetLen [[COPY2]]
-  ; CHECK-NEXT:   [[CGetTag:%[0-9]+]]:gpr = CGetTag [[COPY2]]
   ; CHECK-NEXT:   [[CGetSealed:%[0-9]+]]:gpr = CGetSealed [[COPY2]]
   ; CHECK-NEXT:   [[CGetOffset:%[0-9]+]]:gpr = CGetOffset [[COPY2]]
   ; CHECK-NEXT:   [[PseudoCGetAddr:%[0-9]+]]:gpr = PseudoCGetAddr [[COPY2]]
@@ -142,6 +141,7 @@ body:             |
   ; CHECK-NEXT: bb.3.if.then:
   ; CHECK-NEXT:   successors: %bb.4(0x80000000)
   ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[CGetTag:%[0-9]+]]:gpr = CGetTag [[COPY2]]
   ; CHECK-NEXT:   [[CSeal:%[0-9]+]]:gpcr = CSeal [[COPY1]], [[COPY2]]
   ; CHECK-NEXT:   [[CUnseal:%[0-9]+]]:gpcr = CUnseal [[COPY1]], [[COPY2]]
   ; CHECK-NEXT:   [[CAndPerm4:%[0-9]+]]:gpcr = CAndPerm [[COPY1]], [[COPY]]
@@ -213,7 +213,6 @@ body:             |
   ; TAG-CLEAR-NEXT:   [[CGetType:%[0-9]+]]:gpr = CGetType [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[CGetBase:%[0-9]+]]:gpr = CGetBase [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[CGetLen:%[0-9]+]]:gpr = CGetLen [[COPY2]]
-  ; TAG-CLEAR-NEXT:   [[CGetTag:%[0-9]+]]:gpr = CGetTag [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[CGetSealed:%[0-9]+]]:gpr = CGetSealed [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[CGetOffset:%[0-9]+]]:gpr = CGetOffset [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[PseudoCGetAddr:%[0-9]+]]:gpr = PseudoCGetAddr [[COPY2]]
@@ -302,6 +301,7 @@ body:             |
   ; TAG-CLEAR-NEXT: bb.3.if.then:
   ; TAG-CLEAR-NEXT:   successors: %bb.4(0x80000000)
   ; TAG-CLEAR-NEXT: {{  $}}
+  ; TAG-CLEAR-NEXT:   [[CGetTag:%[0-9]+]]:gpr = CGetTag [[COPY2]]
   ; TAG-CLEAR-NEXT:   [[CSpecialRW:%[0-9]+]]:gpcr = CSpecialRW 0, [[COPY1]]
   ; TAG-CLEAR-NEXT:   CClear 1, 1
   ; TAG-CLEAR-NEXT:   FPClear 1, 1

diff --git a/llvm/test/CodeGen/RISCV/cheri/tag-get-cse.ll b/llvm/test/CodeGen/RISCV/cheri/tag-get-cse.ll
@@ -0,0 +1,31 @@
+; RUN: %riscv32_cheri_purecap_llc < %s | FileCheck %s
+target datalayout = "e-m:e-pf200:64:64:64:32-p:32:32-i64:64-n32-S128-A200-P200-G200"
+target triple = "riscv32-unknown-unknown"
+
+; Function Attrs: nounwind
+define dso_local i32 @foo() local_unnamed_addr addrspace(200) {
+; CHECK-LABEL: foo:
+; CHECK:    cgettag
+; CHECK:    ccall print
+; CHECK:    ccall might_free
+; CHECK:    cgettag
+; CHECK:    ccall print
+; CHECK:    cret
+entry:
+  %call = tail call  dereferenceable_or_null(128) addrspace(200) ptr addrspace(200) @malloc(i32 noundef 128)
+  %0 = tail call addrspace(200) i1 @llvm.cheri.cap.tag.get(ptr addrspace(200) %call)
+  tail call addrspace(200) void @print(i1 noundef zeroext %0) #4
+  tail call addrspace(200) void @might_free(ptr addrspace(200) noundef %call) #4
+  %1 = tail call addrspace(200) i1 @llvm.cheri.cap.tag.get(ptr addrspace(200) %call)
+  tail call addrspace(200) void @print(i1 noundef zeroext %1) #4
+  ret i32 0
+}
+
+declare dso_local  noalias noundef ptr addrspace(200) @malloc(i32 noundef) local_unnamed_addr addrspace(200)
+
+declare i1 @llvm.cheri.cap.tag.get(ptr addrspace(200)) addrspace(200)
+
+declare dso_local void @print(i1 noundef zeroext) local_unnamed_addr addrspace(200)
+
+declare dso_local void @might_free(ptr addrspace(200) noundef) local_unnamed_addr addrspace(200)
+