2
0
Эх сурвалжийг харах

Move HLSL shift behavior to frontend (#4643)

* [NFC] Move HLSL shfit behavior to frontend

The HLSL bit shift behavior was implemented in constant folding which
alters the semantics of the IR. Alternatively we can implement it in
the front end, leveraging the existing OpenCL code paths.

This results in the same final IR optimization and allows reverting the
ConstantFolder and IR tests back to match LLVM 3.7.

The HLSL shift behavior's optimized behavior is verified in existing
tests like the shift-fold.hlsl test, which is unimpacted by this change.

The big motiviation for this change is heading off subtle bugs. All our
tests cover cases where the shift's size is known at compile time. In
those cases the compiler can resolve the masking behavior
automatically. We have _no_ tests, nor compiler changes that impact
shifts that aren't compile-time known.

This means that prior to this change the behavior of shifting by a
compile-time resolvable value and shifting by a non-resolvable value
likely differs. Since backends often load DXIL and treat it as LLVM IR,
this could cause concerning issues.
Chris B 2 жил өмнө
parent
commit
ae87945a3c

+ 3 - 12
lib/IR/ConstantFold.cpp

@@ -1142,24 +1142,15 @@ Constant *llvm::ConstantFoldBinaryInstruction(unsigned Opcode,
       case Instruction::Shl:
         if (C2V.ult(C1V.getBitWidth()))
           return ConstantInt::get(CI1->getContext(), C1V.shl(C2V));
-        // HLSL Change Begins: Shift only uses bottom bitwidth-1 bits
-        return ConstantInt::get(CI1->getContext(), C1V.shl(C2V & APInt(C2V.getBitWidth(), (uint64_t)(C1V.getBitWidth() - 1))));
-        //return UndefValue::get(C1->getType()); // too big shift is undef
-        // HLSL Change Ends
+        return UndefValue::get(C1->getType()); // too big shift is undef
       case Instruction::LShr:
         if (C2V.ult(C1V.getBitWidth()))
           return ConstantInt::get(CI1->getContext(), C1V.lshr(C2V));
-        // HLSL Change Begins: Shift only uses bottom bitwidth-1 bits
-        return ConstantInt::get(CI1->getContext(), C1V.lshr(C2V & APInt(C2V.getBitWidth(), (uint64_t)(C1V.getBitWidth() - 1))));
-        //return UndefValue::get(C1->getType()); // too big shift is undef
-        // HLSL Change Ends
+        return UndefValue::get(C1->getType()); // too big shift is undef
       case Instruction::AShr:
         if (C2V.ult(C1V.getBitWidth()))
           return ConstantInt::get(CI1->getContext(), C1V.ashr(C2V));
-        // HLSL Change Begins: Shift only uses bottom bitwidth-1 bits
-        return ConstantInt::get(CI1->getContext(), C1V.ashr(C2V & APInt(C2V.getBitWidth(), (uint64_t)(C1V.getBitWidth() - 1))));
-        //return UndefValue::get(C1->getType()); // too big shift is undef
-        // HLSL Change Ends
+        return UndefValue::get(C1->getType()); // too big shift is undef
       }
     }
 

+ 2 - 2
tools/clang/lib/CodeGen/CGExprScalar.cpp

@@ -3037,7 +3037,7 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
                       Ops.Ty->hasSignedIntegerRepresentation();
   bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind::ShiftExponent);
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
-  if (CGF.getLangOpts().OpenCL)
+  if (CGF.getLangOpts().OpenCL || CGF.getLangOpts().HLSL) // HLSL Change
     RHS =
         Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shl.mask");
   else if ((SanitizeBase || SanitizeExponent) &&
@@ -3098,7 +3098,7 @@ Value *ScalarExprEmitter::EmitShr(const BinOpInfo &Ops) {
     RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
 
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
-  if (CGF.getLangOpts().OpenCL)
+  if (CGF.getLangOpts().OpenCL || CGF.getLangOpts().HLSL)
     RHS =
         Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shr.mask");
   else if (CGF.SanOpts.has(SanitizerKind::ShiftExponent) &&

+ 0 - 21
tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp

@@ -1793,27 +1793,6 @@ void SimpleTransformForHLDXIRInst(Instruction *I, SmallInstSet &deadInsts) {
       }
     }
   } break;
-  case Instruction::LShr:
-  case Instruction::AShr:
-  case Instruction::Shl: {
-    llvm::BinaryOperator *BO = cast<llvm::BinaryOperator>(I);
-    Value *op2 = BO->getOperand(1);
-    IntegerType *Ty = cast<IntegerType>(BO->getType()->getScalarType());
-    unsigned bitWidth = Ty->getBitWidth();
-    // Clamp op2 to 0 ~ bitWidth-1
-    if (ConstantInt *cOp2 = dyn_cast<ConstantInt>(op2)) {
-      unsigned iOp2 = cOp2->getLimitedValue();
-      unsigned clampedOp2 = iOp2 & (bitWidth - 1);
-      if (iOp2 != clampedOp2) {
-        BO->setOperand(1, ConstantInt::get(op2->getType(), clampedOp2));
-      }
-    } else {
-      Value *mask = ConstantInt::get(op2->getType(), bitWidth - 1);
-      IRBuilder<> Builder(I);
-      op2 = Builder.CreateAnd(op2, mask);
-      BO->setOperand(1, op2);
-    }
-  } break;
   }
 }
 

+ 33 - 0
tools/clang/test/HLSLFileCheck/hlsl/operators/binary/shift-mask.hlsl

@@ -0,0 +1,33 @@
+// RUN: %dxc -T lib_6_3 %s -fcgl | FileCheck %s
+
+int shl32(int V, int S) {
+  return V << S;
+}
+
+// CHECK: define internal i32 @"\01?shl32
+// CHECK-DAG:  %[[Masked:.*]] = and i32 %{{.*}}, 31
+// CHECK-DAG:  %{{.*}} = shl i32 %{{.*}}, %[[Masked]]
+
+int shr32(int V, int S) {
+  return V >> S;
+}
+
+// CHECK: define internal i32 @"\01?shr32
+// CHECK-DAG:  %[[Masked:.*]] = and i32 %{{.*}}, 31
+// CHECK-DAG:  %{{.*}} = ashr i32 %{{.*}}, %[[Masked]]
+
+int64_t shl64(int64_t V, int64_t S) {
+  return V << S;
+}
+
+// CHECK define internal i64 @"\01?shl64
+// CHECK-DAG:  %[[Masked:.*]] = and i64 %{{.*}}, 63
+// CHECK-DAG:  %{{.*}} = shl i64 %{{.*}}, %[[Masked]]
+
+int64_t shr64(int64_t V, int64_t S) {
+  return V >> S;
+}
+
+// CHECK: define internal i64 @"\01?shr64@@YA_J_J0@Z"(i64 %V, i64 %S) #0 {
+// CHECK-DAG:  %[[Masked:.*]] = and i64 %{{.*}}, 63
+// CHECK-DAG:  %{{.*}} = ashr i64 %{{.*}}, %[[Masked]]

+ 1 - 1
unittests/IR/ConstantsTest.cpp

@@ -26,7 +26,7 @@ TEST(ConstantsTest, Integer_i1) {
   Constant* Zero = ConstantInt::get(Int1, 0);
   Constant* NegOne = ConstantInt::get(Int1, static_cast<uint64_t>(-1), true);
   EXPECT_EQ(NegOne, ConstantInt::getSigned(Int1, -1));
-  Constant* Undef = One; // UndefValue::get(Int1) -  HLSL Change
+  Constant* Undef = UndefValue::get(Int1);
 
   // Input:  @b = constant i1 add(i1 1 , i1 1)
   // Output: @b = constant i1 false