canCreateUndefOrPoison: dyn_cast -> dyn_cast_or_null

[markus]

Summary

Trivial change of a dyn_cast to dyn_cast_or_null in ValueTracking.

[markus]

bbi-51646.ll1 KBDownload

The issue was found with attached input bbi-51646.ll If necessary it could be added as a regression test but then the question is where (i.e. should it piggyback on some other file).

llvm/lib/Analysis/ValueTracking.cpp

4740

Constant::getAggregateElement() at line 4733 may return nullptr.

[fhahn]

In D94494#2492801, @markus wrote:

bbi-51646.ll1 KBDownload

The issue was found with attached input bbi-51646.ll If necessary it could be added as a regression test but then the question is where (i.e. should it piggyback on some other file).

Yes, please add a regression test.

[dblaikie]

In D94494#2492801, @markus wrote:

bbi-51646.ll1 KBDownload

The issue was found with attached input bbi-51646.ll If necessary it could be added as a regression test but then the question is where (i.e. should it piggyback on some other file).

Probably just in its own file (we don't do a very good job of keeping a lot of related tests in a single file together - usually just add more files, it's not ideal but often hard to find other good groupings) - but if you're looking for related test files, try breaking the pass in some obvious way and see what tests fail - that should help you find some related tests that could be piggy backed on.

[dblaikie]

In D94494#2493719, @dblaikie wrote:

In D94494#2492801, @markus wrote:

bbi-51646.ll1 KBDownload

The issue was found with attached input bbi-51646.ll If necessary it could be added as a regression test but then the question is where (i.e. should it piggyback on some other file).

Probably just in its own file (we don't do a very good job of keeping a lot of related tests in a single file together - usually just add more files, it's not ideal but often hard to find other good groupings) - but if you're looking for related test files, try breaking the pass in some obvious way

Such as adding assert(false) in the codepath you're experimenting with, etc.

and see what tests fail - that should help you find some related tests that could be piggy backed on.

[aqjune]

Thanks!
Yes, a test should be added.
I guess

ashr <4 x i16> %induction, select (i1 icmp sgt (i16 ptrtoint (i16* @c to i16), i16 1), <4 x i16> zeroinitializer, <4 x i16> <i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16)>)

is the problematic part?
If it is non-trivial to further reduce the input, you can also add this instruction to the list at ValueTrackingTest.cpp 's TEST(ValueTracking, canCreatePoisonOrUndef).

[markus]

Added test.

[markus]

In D94494#2494746, @aqjune wrote:

Thanks!
Yes, a test should be added.
I guess

ashr <4 x i16> %induction, select (i1 icmp sgt (i16 ptrtoint (i16* @c to i16), i16 1), <4 x i16> zeroinitializer, <4 x i16> <i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16), i16 ptrtoint (i16* @c to i16)>)

is the problematic part?
If it is non-trivial to further reduce the input, you can also add this instruction to the list at ValueTrackingTest.cpp 's TEST(ValueTracking, canCreatePoisonOrUndef).

Yes that is the problematic part. I think it makes most sense to add as a targeted unit-test for the specific function as below. Unfortunately the IR line is a bit messy but I was not able to simplify it any further. I guess it needs to be something that is considered a Constant but yet does not simplify to an "actual" Constant during import.

[aqjune]

LGTM

llvm/lib/Analysis/ValueTracking.cpp

if (auto *C = dyn_cast<Constant>(Op->getOperand(1))) {
         for (unsigned i = 0; i < NumElts; ++i)
           ShiftAmounts.push_back(C->getAggregateElement(i));
       } else if (isa<ScalableVectorType>(C->getType()))
         return true; // Can't tell, just return true to be safe
       else
         ShiftAmounts.push_back(C);
 
       bool Safe = llvm::all_of(ShiftAmounts, [](Constant *C) {
-        auto *CI = dyn_cast<ConstantInt>(C);
+        auto *CI = dyn_cast_or_null<ConstantInt>(C);
         return CI && CI->getValue().ult(C->getType()->getIntegerBitWidth());
       });
       return !Safe;
     }
     return true;
   }
   case Instruction::FPToSI:
   case Instruction::FPToUI:

llvm/unittests/Analysis/ValueTrackingTest.cpp

TEST_F(ValueTrackingTest, isGuaranteedNotToBeUndefOrPoison_assume) {
   DominatorTree DT(*F);
   EXPECT_FALSE(isGuaranteedNotToBeUndefOrPoison(A, &AC, CxtI, &DT));
   EXPECT_FALSE(isGuaranteedNotToBeUndefOrPoison(A, &AC, CxtI2, &DT));
   EXPECT_TRUE(isGuaranteedNotToBeUndefOrPoison(A, &AC, CxtI3, &DT));
 }
 
 TEST(ValueTracking, canCreatePoisonOrUndef) {
   std::string AsmHead =
+      "@s = external dso_local global i32, align 1\n"
       "declare i32 @g(i32)\n"
       "define void @f(i32 %x, i32 %y, float %fx, float %fy, i1 %cond, "
       "<4 x i32> %vx, <4 x i32> %vx2, <vscale x 4 x i32> %svx, i8* %p) {\n";
   std::string AsmTail = "  ret void\n}";
   // (can create poison?, can create undef?, IR instruction)
   SmallVector<std::pair<std::pair<bool, bool>, std::string>, 32> Data = {
       {{false, false}, "add i32 %x, %y"},
       {{true, false}, "add nsw nuw i32 %x, %y"},
SmallVector<std::pair<std::pair<bool, bool>, std::string>, 32> Data = {
        "shufflevector <4 x i32> %vx, <4 x i32> %vx2, "
        "<4 x i32> <i32 0, i32 1, i32 2, i32 undef>"},
       {{false, true},
        "shufflevector <vscale x 4 x i32> %svx, "
        "<vscale x 4 x i32> %svx, <vscale x 4 x i32> undef"},
       {{true, false}, "call i32 @g(i32 %x)"},
       {{false, false}, "call noundef i32 @g(i32 %x)"},
       {{true, false}, "fcmp nnan oeq float %fx, %fy"},
-      {{false, false}, "fcmp oeq float %fx, %fy"}};
+      {{false, false}, "fcmp oeq float %fx, %fy"},
+      {{true, false},
+       "ashr <4 x i32> %vx, select (i1 icmp sgt (i32 ptrtoint (i32* @s to "
+       "i32), i32 1), <4 x i32> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 "
+       "2, i32 3>)"}};
 
   std::string AssemblyStr = AsmHead;
   for (auto &Itm : Data)
     AssemblyStr += Itm.second + "\n";
   AssemblyStr += AsmTail;
 
   LLVMContext Context;
   SMDiagnostic Error;