Skip to content

Conversation

@resistor
Copy link
Collaborator

@resistor resistor commented Dec 9, 2024

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Owen Anderson (resistor)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/119156.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/VNCoercion.cpp (+5)
  • (added) llvm/test/Transforms/GVN/nonintegral-ptr-coercion.ll (+45)
diff --git a/llvm/lib/Transforms/Utils/VNCoercion.cpp b/llvm/lib/Transforms/Utils/VNCoercion.cpp
index 1e0ae280516410..ebccaf8da7ba8d 100644
--- a/llvm/lib/Transforms/Utils/VNCoercion.cpp
+++ b/llvm/lib/Transforms/Utils/VNCoercion.cpp
@@ -27,6 +27,11 @@ bool canCoerceMustAliasedValueToLoad(Value *StoredVal, Type *LoadTy,
       isFirstClassAggregateOrScalableType(StoredTy))
     return false;
 
+  // We aren't allowed to introduce integer casts of non-integral pointers.
+  if (DL.isNonIntegralPointerType(LoadTy) ||
+      DL.isNonIntegralPointerType(StoredTy))
+    return false;
+
   uint64_t StoreSize = DL.getTypeSizeInBits(StoredTy).getFixedValue();
 
   // The store size must be byte-aligned to support future type casts.
diff --git a/llvm/test/Transforms/GVN/nonintegral-ptr-coercion.ll b/llvm/test/Transforms/GVN/nonintegral-ptr-coercion.ll
new file mode 100644
index 00000000000000..b4c7450cf15ae1
--- /dev/null
+++ b/llvm/test/Transforms/GVN/nonintegral-ptr-coercion.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --scrub-attributes --version 5
+; RUN: opt -S -passes=gvn < %s | FileCheck %s
+source_filename = "/app/example.cpp"
+target datalayout = "e-m:e-p:32:32-p200:128:128:128:64-i64:64-n32-S128-ni:200"
+target triple = "riscv32-unknown-linux-gnu"
+
+; Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite, inaccessiblemem: readwrite) uwtable
+define dso_local noundef zeroext i8 @test(ptr addrspace(200) noundef initializes((0, 4)) %0) local_unnamed_addr #0 {
+; CHECK-LABEL: define dso_local noundef zeroext i8 @test(
+; CHECK-SAME: ptr addrspace(200) noundef initializes((0, 4)) [[TMP0:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:    store ptr addrspace(200) null, ptr addrspace(200) [[TMP0]], align 4, !tbaa [[TBAA9:![0-9]+]]
+; CHECK-NEXT:    ret i8 0
+;
+  store ptr addrspace(200) null, ptr addrspace(200) %0, align 4, !tbaa !9
+  %2 = load i8, ptr addrspace(200) %0, align 1, !tbaa !14
+  ret i8 %2
+}
+
+attributes #0 = { mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite, inaccessiblemem: readwrite) uwtable "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic-rv32" "target-features"="+32bit,+a,+c,+d,+f,+m,+relax,+zaamo,+zalrsc,+zicsr,+zifencei,+zmmul,-b,-e,-experimental-smctr,-experimental-ssctr,-experimental-svukte,-experimental-xqcia,-experimental-xqcicsr,-experimental-xqcisls,-experimental-zalasr,-experimental-zicfilp,-experimental-zicfiss,-experimental-zvbc32e,-experimental-zvkgs,-h,-sha,-shcounterenw,-shgatpa,-shtvala,-shvsatpa,-shvstvala,-shvstvecd,-smaia,-smcdeleg,-smcsrind,-smdbltrp,-smepmp,-smmpm,-smnpm,-smrnmi,-smstateen,-ssaia,-ssccfg,-ssccptr,-sscofpmf,-sscounterenw,-sscsrind,-ssdbltrp,-ssnpm,-sspm,-ssqosid,-ssstateen,-ssstrict,-sstc,-sstvala,-sstvecd,-ssu64xl,-supm,-svade,-svadu,-svbare,-svinval,-svnapot,-svpbmt,-svvptc,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfcease,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xsifivecdiscarddlone,-xsifivecflushdlone,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-xwchc,-za128rs,-za64rs,-zabha,-zacas,-zama16b,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmop,-zcmp,-zcmt,-zdinx,-zfa,-zfbfmin,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zic64b,-zicbom,-zicbop,-zicboz,-ziccamoa,-ziccif,-zicclsm,-ziccrse,-zicntr,-zicond,-zihintntl,-zihintpause,-zihpm,-zimop,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-ztso,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfbfmin,-zvfbfwma,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
+
+!llvm.module.flags = !{!0, !1, !2, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"target-abi", !"ilp32d"}
+!2 = !{i32 6, !"riscv-isa", !3}
+!3 = !{!"rv32i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zmmul1p0_zaamo1p0_zalrsc1p0"}
+!4 = !{i32 8, !"PIC Level", i32 2}
+!5 = !{i32 7, !"PIE Level", i32 2}
+!6 = !{i32 7, !"uwtable", i32 2}
+!7 = !{i32 8, !"SmallDataLimit", i32 0}
+!8 = !{!"clang version 20.0.0git (https://github.com/llvm/llvm-project.git 7787328dd64c750c7acf30b86b31f0d7166c8f27)"}
+!9 = !{!10, !10, i64 0}
+!10 = !{!"p1 void", !11, i64 0}
+!11 = !{!"any pointer", !12, i64 0}
+!12 = !{!"omnipotent char", !13, i64 0}
+!13 = !{!"Simple C++ TBAA"}
+!14 = !{!12, !12, i64 0}
+;.
+; CHECK: [[TBAA9]] = !{[[META10:![0-9]+]], [[META10]], i64 0}
+; CHECK: [[META10]] = !{!"p1 void", [[META11:![0-9]+]], i64 0}
+; CHECK: [[META11]] = !{!"any pointer", [[META12:![0-9]+]], i64 0}
+; CHECK: [[META12]] = !{!"omnipotent char", [[META13:![0-9]+]], i64 0}
+; CHECK: [[META13]] = !{!"Simple C++ TBAA"}
+;.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test contains a lot of unnecessary stuff. Also, I'm not sure it's actually testing what you want? The test case still produces an integer for the load. (Though I don't think you'd actually want to prevent it in that case.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary intent is to prevent forwarding of a store of an non-integral pointer to a load of an integer, due to this phase from LangRef: "Because the bit-representation of a non-integral pointer may not be stable, two identical casts of the same operand may or may not return the same value."

I think the test is checking what it's meant to test?

@resistor
Copy link
Collaborator Author

resistor commented Dec 9, 2024

Taking this one back to the drawing board while I investigate the other failures.

@resistor resistor closed this Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants