Make vector operation with variable index unsafe to speculate

[arsenm]

Summary

A branch condition could be guarding an out of bounds access of the vector.

[reames]

It's not clear this is the right model for this. I agree we have to consider out of bounds indices (as specified by the LangRef), but perhaps we'd be better off treating the result as undef/poison as we do for other bits of undefined behaviour? I'm not really the right person to help debate this; you might want to get Sanjoy or David Majnemer involved. They tend to be really good with this family of issues.

[efriedma]

LangRef says "If idx exceeds the length of val, the results are undefined." That implies that the operation itself isn't undefined behavior, and therefore it's safe to speculate.

That said, the current implementation of DAGTypeLegalizer::GetVectorElementPointer can lead to undefined behavior... so we need to either fix that to mask the index, or change LangRef. I would lean towards fixing GetVectorElementPointer.

lib/Analysis/ValueTracking.cpp

3239

isa<Constant> doesn't prove anything useful.

[arsenm]

D26174 makes sure the index is in bounds when legalizing

[arsenm]

The legalization of variable indexes doesn't introduce undefined behavior now

lib/Analysis/ValueTracking.cpp

if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
       // TODO: are convert_{from,to}_fp16 safe?
       // TODO: can we list target-specific intrinsics here?
       default: break;
       }
     }
     return false; // The called function could have undefined behavior or
                   // side-effects, even if marked readnone nounwind.
   }
+  case Instruction::ExtractElement:
+    return isa<Constant>(Inst->getOperand(1));
+  case Instruction::InsertElement:
+    return isa<Constant>(Inst->getOperand(2));
   case Instruction::VAArg:
   case Instruction::Alloca:
   case Instruction::Invoke:
   case Instruction::PHI:
   case Instruction::Store:
   case Instruction::Ret:
   case Instruction::Br:
   case Instruction::IndirectBr:

test/Transforms/SimplifyCFG/speculate-vector-ops.ll

 cond.else27:                                      ; preds = %cond.end18
   br label %cond.end28
 
 cond.end28:                                       ; preds = %cond.else27, %cond.then26
   %cond32 = phi i32 [ %tmp30, %cond.then26 ], [ %cond22, %cond.else27 ]
   br label %return
 }
 
+; CHECK-LABEL: @no_speculate_extractelement_varindex(
+; CHECK: br i1 %cmp
+; CHECK: extractelement
+; CHECK: phi
+define i32 @no_speculate_extractelement_varindex(i32 %c, i32 %idx) #0 {
+entry:
+  %cmp = icmp eq i32 %c, 0
+  br i1 %cmp, label %a, label %b
+
+a:
+  %x = extractelement <4 x i32> undef, i32 %idx
+  br label %b
+
+b:
+  %phi = phi i32 [ 0, %entry], [ %x, %a ]
+  ret i32 %phi
+}
+
+; CHECK-LABEL: @no_speculate_insertelement_varindex(
+; CHECK: br i1 %cmp
+; CHECK: insertelement
+; CHECK: phi
+define <4 x i32> @no_speculate_insertelement_varindex(i32 %c, i32 %idx) #0 {
+entry:
+  %cmp = icmp eq i32 %c, 0
+  br i1 %cmp, label %a, label %b
+
+a:
+  %x = insertelement <4 x i32> undef, i32 8, i32 %idx
+  br label %b
+
+b:
+  %phi = phi <4 x i32> [ zeroinitializer, %entry], [ %x, %a ]
+  ret <4 x i32> %phi
+}
+
 attributes #0 = { nounwind }