[TargetLowering] remove fdiv and frem from canOpTrap() (PR29114)

[spatel]

Summary

Assuming the default FP env, we should not treat fdiv and frem any differently in terms of trapping behavior than any other FP op. Ie, FP ops do not trap with the default FP env.

This matches how we treat these ops in IR with isSafeToSpeculativelyExecute(). If this patch is correct, there's a similar bug in Constant::canTrap().

This bug manifests in PR29114:
https://llvm.org/bugs/show_bug.cgi?id=29114
...as a sequence of scalar divisions instead of a vector division on x86 for a <3 x float> type.

[hfinkel]

LGTM

[spatel]

Thanks, Hal. I committed the related change to Constant::canTrap() at rL279973 .

include/llvm/Target/TargetLowering.h

public:
   virtual bool isShuffleMaskLegal(const SmallVectorImpl<int> &/*Mask*/,
                                   EVT /*VT*/) const {
     return true;
   }
 
   /// Returns true if the operation can trap for the value type.
   ///
   /// VT must be a legal type. By default, we optimistically assume most
-  /// operations don't trap except for divide and remainder.
+  /// operations don't trap except for integer divide and remainder.
   virtual bool canOpTrap(unsigned Op, EVT VT) const;
 
   /// Similar to isShuffleMaskLegal. This is used by Targets can use this to
   /// indicate if there is a suitable VECTOR_SHUFFLE that can be used to replace
   /// a VAND with a constant pool entry.
   virtual bool isVectorClearMaskLegal(const SmallVectorImpl<int> &/*Mask*/,
                                       EVT /*VT*/) const {
     return false;

lib/CodeGen/TargetLoweringBase.cpp

 EVT TargetLoweringBase::getShiftAmountTy(EVT LHSTy,
                                          const DataLayout &DL) const {
   assert(LHSTy.isInteger() && "Shift amount is not an integer type!");
   if (LHSTy.isVector())
     return LHSTy;
   return getScalarShiftAmountTy(DL, LHSTy);
 }
 
-/// canOpTrap - Returns true if the operation can trap for the value type.
-/// VT must be a legal type.
 bool TargetLoweringBase::canOpTrap(unsigned Op, EVT VT) const {
   assert(isTypeLegal(VT));
   switch (Op) {
   default:
     return false;
-  case ISD::FDIV:
-  case ISD::FREM:
   case ISD::SDIV:
   case ISD::UDIV:
   case ISD::SREM:
   case ISD::UREM:
     return true;
   }
 }
 

test/CodeGen/X86/vec3.ll

;
   ret <3 x float> %add
 }
 
 ; PR29114: https://llvm.org/bugs/show_bug.cgi?id=29114
 
 define <3 x float> @fdiv(<3 x float> %v, float %d) {
 ; CHECK-LABEL: fdiv:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    movaps %xmm1, %xmm2
-; CHECK-NEXT:    movaps %xmm0, %xmm3
-; CHECK-NEXT:    movaps %xmm1, %xmm4
-; CHECK-NEXT:    divss %xmm0, %xmm1
-; CHECK-NEXT:    shufps {{.*#+}} xmm0 = xmm0[1,1,2,3]
-; CHECK-NEXT:    divss %xmm0, %xmm2
-; CHECK-NEXT:    movhlps {{.*#+}} xmm3 = xmm3[1,1]
-; CHECK-NEXT:    divss %xmm3, %xmm4
-; CHECK-NEXT:    unpcklps {{.*#+}} xmm1 = xmm1[0],xmm4[0],xmm1[1],xmm4[1]
-; CHECK-NEXT:    unpcklps {{.*#+}} xmm1 = xmm1[0],xmm2[0],xmm1[1],xmm2[1]
+; CHECK-NEXT:    shufps {{.*#+}} xmm1 = xmm1[0,0,0,3]
+; CHECK-NEXT:    divps %xmm0, %xmm1
 ; CHECK-NEXT:    movaps %xmm1, %xmm0
 ; CHECK-NEXT:    retq
 ;
   %ins = insertelement <3 x float> undef, float %d, i32 0
   %splat = shufflevector <3 x float> %ins, <3 x float> undef, <3 x i32> zeroinitializer
   %div = fdiv <3 x float> %splat, %v
   ret <3 x float> %div
 }