[PATCH] D115247: [EarlyCSE] Retain poison flags, if program is UB if poison.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 10 08:06:24 PST 2021


fhahn updated this revision to Diff 393497.
fhahn added a comment.

In D115247#3178708 <https://reviews.llvm.org/D115247#3178708>, @nikic wrote:

>> Note that the code as written assumes that an instruction cannot have
>> poison-generating flags and fast-math flags, but I am not sure this is
>> always true. Should we add a version of andIRFlags that selectively
>> allows AND'ing fast-math flags only?
>
> Yes, I think so. Or more specifically, to only and the non-poison-generating flags. There are some FMF that are poison generating, but we don't currently treat them as such in various "poison generating" queries. If that changed in the future, we should still handle it correctly.

I split out the code to 'and' non-poison generating flags in D115527 <https://reviews.llvm.org/D115527> and update the patch to use it in case the program is not UB if I is poison.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115247/new/

https://reviews.llvm.org/D115247

Files:
  llvm/lib/Transforms/Scalar/EarlyCSE.cpp
  llvm/test/Transforms/EarlyCSE/flags.ll


Index: llvm/test/Transforms/EarlyCSE/flags.ll
===================================================================
--- llvm/test/Transforms/EarlyCSE/flags.ll
+++ llvm/test/Transforms/EarlyCSE/flags.ll
@@ -22,7 +22,7 @@
 
 define void @test_inbounds_program_ub_if_first_gep_poison(i8* %ptr, i64 %n) {
 ; CHECK-LABEL: @test_inbounds_program_ub_if_first_gep_poison(
-; CHECK-NEXT:    [[ADD_PTR_1:%.*]] = getelementptr i8, i8* [[PTR:%.*]], i64 [[N:%.*]]
+; CHECK-NEXT:    [[ADD_PTR_1:%.*]] = getelementptr inbounds i8, i8* [[PTR:%.*]], i64 [[N:%.*]]
 ; CHECK-NEXT:    call void @use.i8(i8* noundef [[ADD_PTR_1]])
 ; CHECK-NEXT:    call void @use.i8(i8* [[ADD_PTR_1]])
 ; CHECK-NEXT:    ret void
Index: llvm/lib/Transforms/Scalar/EarlyCSE.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1366,8 +1366,13 @@
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
-        if (auto *I = dyn_cast<Instruction>(V))
-          I->andIRFlags(&Inst);
+        if (auto *I = dyn_cast<Instruction>(V)) {
+          if (programUndefinedIfPoison(I)) {
+            // Keep poison generating flags, if I being poison is UB.
+            I->andNonPoisonGeneratingIRFlags(&Inst);
+          } else
+            I->andIRFlags(&Inst);
+        }
         Inst.replaceAllUsesWith(V);
         salvageKnowledge(&Inst, &AC);
         removeMSSA(Inst);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115247.393497.patch
Type: text/x-patch
Size: 1487 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211210/c03dd5e2/attachment.bin>


More information about the llvm-commits mailing list