[InstCombine][sse4a] Fix assertion failure caused by unsafe dyn_casts on the operands of extrq/extrqi intrinsic calls.

[andreadb]

Summary

This patch fixes an assertion failure caused by unsafe dynamic casts on the constant operands of sse4a intrinsic calls to extrq/extrqi.

The combine logic that simplifies sse4a extrq/extrqi intrinsic calls currently checks if the input operands are constants. Internally, the logic relies on unsafe dyn_casts on the values returned by calls to method Constant::getAggregateElement.
However, method 'getAggregateElement ' may return null if the constant element cannot be retrieved. So all the dyn_cast can potentially fail. This is what happens for example if a constexpr values is passed in input to the extrq/extrqi intrinsic calls.

I have added two test cases that fail with the following assertion failure:

include/llvm/Support/Casting.h:95: static bool
llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::ConstantInt; From =
llvm::Constant]: Assertion `Val && "isa<> used on a null pointer"' failed

This patch fixes the problem by using a dyn_cast_or_null (instead of a simple dyn_cast on the result of calls to Constant::getAggregateElement.

Please let me know if okay to commit.

Thanks,
Andrea

[RKSimon]

Thanks Andrea, does INSERTQI have a similar problem?

[andreadb]

In D24256#535193, @RKSimon wrote:

Thanks Andrea, does INSERTQI have a similar problem?

Possibly. We are currently running extensive tests. So far, the extrq is the only problematic one. I wouldn't be surprised if that happens with the INSERTQI though (I saw that there is similar code in there). I will run some experiments and I will let you know in case.
Would it be okay if in the meatime I commit this change?

Have a nice day,
Andrea

[RKSimon]

LGTM as long as you check if INSERTQ/INSERTQI is affected as well, if so you're probably good to just commit the equivalent fix/tests as a followup.

[andreadb]

In D24256#535655, @RKSimon wrote:

LGTM as long as you check if INSERTQ/INSERTQI is affected as well, if so you're probably good to just commit the equivalent fix/tests as a followup.

Sure, I will test INSERTQ/INSERTQI as well.

Thanks Simon

llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp

auto LowConstantHighUndef = [&](uint64_t Val) {
     Constant *Args[] = {ConstantInt::get(IntTy64, Val),
                         UndefValue::get(IntTy64)};
     return ConstantVector::get(Args);
   };
 
   // See if we're dealing with constant values.
   Constant *C0 = dyn_cast<Constant>(Op0);
   ConstantInt *CI0 =
-      C0 ? dyn_cast<ConstantInt>(C0->getAggregateElement((unsigned)0))
+      C0 ? dyn_cast_or_null<ConstantInt>(C0->getAggregateElement((unsigned)0))
          : nullptr;
 
   // Attempt to constant fold.
   if (CILength && CIIndex) {
     // From AMD documentation: "The bit index and field length are each six
     // bits in length other bits of the field are ignored."
     APInt APIndex = CIIndex->getValue().zextOrTrunc(6);
     APInt APLength = CILength->getValue().zextOrTrunc(6);
case Intrinsic::x86_sse4a_extrq: {
     unsigned VWidth1 = Op1->getType()->getVectorNumElements();
     assert(Op0->getType()->getPrimitiveSizeInBits() == 128 &&
            Op1->getType()->getPrimitiveSizeInBits() == 128 && VWidth0 == 2 &&
            VWidth1 == 16 && "Unexpected operand sizes");
 
     // See if we're dealing with constant values.
     Constant *C1 = dyn_cast<Constant>(Op1);
     ConstantInt *CILength =
-        C1 ? dyn_cast<ConstantInt>(C1->getAggregateElement((unsigned)0))
+        C1 ? dyn_cast_or_null<ConstantInt>(C1->getAggregateElement((unsigned)0))
            : nullptr;
     ConstantInt *CIIndex =
-        C1 ? dyn_cast<ConstantInt>(C1->getAggregateElement((unsigned)1))
+        C1 ? dyn_cast_or_null<ConstantInt>(C1->getAggregateElement((unsigned)1))
            : nullptr;
 
     // Attempt to simplify to a constant, shuffle vector or EXTRQI call.
     if (Value *V = simplifyX86extrq(*II, Op0, CILength, CIIndex, *Builder))
       return replaceInstUsesWith(*II, V);
 
     // EXTRQ only uses the lowest 64-bits of the first 128-bit vector
     // operands and the lowest 16-bits of the second.

llvm/trunk/test/Transforms/InstCombine/x86-sse4a.ll

 define <2 x i64> @test_extrq_constant_undef(<2 x i64> %x, <16 x i8> %y) {
 ; CHECK-LABEL: @test_extrq_constant_undef(
 ; CHECK-NEXT:    ret <2 x i64> <i64 65535, i64 undef>
 ;
   %1 = tail call <2 x i64> @llvm.x86.sse4a.extrq(<2 x i64> <i64 -1, i64 undef>, <16 x i8> <i8 16, i8 15, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>) nounwind
   ret <2 x i64> %1
 }
 
+define <2 x i64> @test_extrq_call_constexpr(<2 x i64> %x) {
+; CHECK-LABEL: @test_extrq_call_constexpr(
+; CHECK-NEXT:    [[TMP1:%.*]] = call <2 x i64> @llvm.x86.sse4a.extrq(<2 x i64> %x, <16 x i8> bitcast (<2 x i64> <i64 0, i64 undef> to <16 x i8>))
+; CHECK-NEXT:    ret <2 x i64> [[TMP1]]
+;
+  %1 = call <2 x i64> @llvm.x86.sse4a.extrq(<2 x i64> %x, <16 x i8> bitcast (<2 x i64> <i64 0, i64 undef> to <16 x i8>))
+  ret <2 x i64> %1
+}
+
 ;
 ; EXTRQI
 ;
 
 define <2 x i64> @test_extrqi_call(<2 x i64> %x) {
 ; CHECK-LABEL: @test_extrqi_call(
 ; CHECK-NEXT:    [[TMP1:%.*]] = tail call <2 x i64> @llvm.x86.sse4a.extrqi(<2 x i64> %x, i8 8, i8 23)
 ; CHECK-NEXT:    ret <2 x i64> [[TMP1]]

 define <2 x i64> @test_extrqi_constant_undef(<2 x i64> %x) {
 ; CHECK-LABEL: @test_extrqi_constant_undef(
 ; CHECK-NEXT:    ret <2 x i64> <i64 15, i64 undef>
 ;
   %1 = tail call <2 x i64> @llvm.x86.sse4a.extrqi(<2 x i64> <i64 -1, i64 undef>, i8 4, i8 18)
   ret <2 x i64> %1
 }
 
+define <2 x i64> @test_extrqi_call_constexpr() {
+; CHECK-LABEL: @test_extrqi_call_constexpr(
+; CHECK-NEXT:    ret <2 x i64> bitcast (<16 x i8> <i8 extractelement (<16 x i8> trunc (<16 x i16> bitcast (<4 x i64> <i64 0, i64 undef, i64 2, i64 undef> to <16 x i16>) to <16 x i8>), i32 2), i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef> to <2 x i64>)
+;
+  %1 = tail call <2 x i64> @llvm.x86.sse4a.extrqi(<2 x i64> bitcast (<16 x i8> trunc (<16 x i16> bitcast (<4 x i64> <i64 0, i64 undef, i64 2, i64 undef> to <16 x i16>) to <16 x i8>) to <2 x i64>), i8 8, i8 16)
+  ret <2 x i64> %1
+}
+
 ;
 ; INSERTQ
 ;
 
 define <2 x i64> @test_insertq_call(<2 x i64> %x, <2 x i64> %y) {
 ; CHECK-LABEL: @test_insertq_call(
 ; CHECK-NEXT:    [[TMP1:%.*]] = tail call <2 x i64> @llvm.x86.sse4a.insertq(<2 x i64> %x, <2 x i64> %y) #1
 ; CHECK-NEXT:    ret <2 x i64> [[TMP1]]