Revert "Use Sign/ZeroExtend image operands for read/write_image"

This reverts commit 4256e4e. This
change enabled a test that is failing on Windows, so I'm leaving it out
of this week's pulldown.

Author: dwoodwor-intel

llvm-spirv/lib/SPIRV/OCLToSPIRV.cpp

@@ -84,20 +84,6 @@ static Type *getBlockStructType(Value *Parameter) {
   return ParamType;
 }
 
-/// Return one of the SPIR-V 1.4 SignExtend or ZeroExtend image operands
-/// for a demangled function name, or 0 if the function does not return an
-/// integer type (e.g. read_imagef).
-static unsigned getImageSignZeroExt(StringRef DemangledName) {
-  bool IsSigned = !DemangledName.endswith("ui") && DemangledName.back() == 'i';
-  bool IsUnsigned = DemangledName.endswith("ui");
-
-  if (IsSigned)
-    return ImageOperandsMask::ImageOperandsSignExtendMask;
-  if (IsUnsigned)
-    return ImageOperandsMask::ImageOperandsZeroExtendMask;
-  return 0;
-}
-
 bool OCLToSPIRVLegacy::runOnModule(Module &M) {
   setOCLTypeToSPIRV(&getAnalysis<OCLTypeToSPIRVLegacy>());
   return runOCLToSPIRV(M);
@@ -280,7 +266,7 @@ void OCLToSPIRVBase::visitCallInst(CallInst &CI) {
   }
   if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0) {
     if (MangledName.find(kMangledName::Sampler) != StringRef::npos) {
-      visitCallReadImageWithSampler(&CI, MangledName, DemangledName);
+      visitCallReadImageWithSampler(&CI, MangledName);
       return;
     }
     if (MangledName.find("msaa") != StringRef::npos) {
@@ -987,8 +973,7 @@ void OCLToSPIRVBase::visitCallReadImageMSAA(CallInst *CI,
 }
 
 void OCLToSPIRVBase::visitCallReadImageWithSampler(CallInst *CI,
-                                                   StringRef MangledName,
-                                                   StringRef DemangledName) {
+                                                   StringRef MangledName) {
   assert(MangledName.find(kMangledName::Sampler) != StringRef::npos);
   assert(CI->getCalledFunction() && "Unexpected indirect call");
   Function *Func = CI->getCalledFunction();
@@ -1014,26 +999,22 @@ void OCLToSPIRVBase::visitCallReadImageWithSampler(CallInst *CI,
         Args[0] = SampledImg;
         Args.erase(Args.begin() + 1, Args.begin() + 2);
 
-        unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
-        unsigned ImgOpMaskInsIndex = Args.size();
         switch (Args.size()) {
         case 2: // no lod
-          ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
-          ImgOpMaskInsIndex = Args.size();
+          Args.push_back(getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
           Args.push_back(getFloat32(M, 0.f));
           break;
         case 3: // explicit lod
-          ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
-          ImgOpMaskInsIndex = 2;
+          Args.insert(Args.begin() + 2,
+                      getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
           break;
         case 4: // gradient
-          ImgOpMask |= ImageOperandsMask::ImageOperandsGradMask;
-          ImgOpMaskInsIndex = 2;
+          Args.insert(Args.begin() + 2,
+                      getInt32(M, ImageOperandsMask::ImageOperandsGradMask));
           break;
         default:
           assert(0 && "read_image* with unhandled number of args!");
         }
-        Args.insert(Args.begin() + ImgOpMaskInsIndex, getInt32(M, ImgOpMask));
 
         // SPIR-V instruction always returns 4-element vector
         if (IsRetScalar)
@@ -1149,32 +1130,19 @@ void OCLToSPIRVBase::visitCallBuiltinSimple(CallInst *CI, StringRef MangledName,
 void OCLToSPIRVBase::visitCallReadWriteImage(CallInst *CI,
                                              StringRef DemangledName) {
   OCLBuiltinTransInfo Info;
-  if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0) {
+  if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0)
     Info.UniqName = kOCLBuiltinName::ReadImage;
-    unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
-    if (ImgOpMask) {
-      Info.PostProc = [&](std::vector<Value *> &Args) {
-        Args.push_back(getInt32(M, ImgOpMask));
-      };
-    }
-  }
 
   if (DemangledName.find(kOCLBuiltinName::WriteImage) == 0) {
     Info.UniqName = kOCLBuiltinName::WriteImage;
     Info.PostProc = [&](std::vector<Value *> &Args) {
-      unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
-      unsigned ImgOpMaskInsIndex = Args.size();
       if (Args.size() == 4) // write with lod
       {
         auto Lod = Args[2];
         Args.erase(Args.begin() + 2);
-        ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
-        ImgOpMaskInsIndex = Args.size();
+        Args.push_back(getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
         Args.push_back(Lod);
       }
-      if (ImgOpMask) {
-        Args.insert(Args.begin() + ImgOpMaskInsIndex, getInt32(M, ImgOpMask));
-      }
     };
   }
 

llvm-spirv/lib/SPIRV/OCLToSPIRV.h

@@ -169,8 +169,7 @@ class OCLToSPIRVBase : public InstVisitor<OCLToSPIRVBase> {
   /// read_image(image, sampler, ...) =>
   ///   sampled_image = __spirv_SampledImage(image, sampler);
   ///   return __spirv_ImageSampleExplicitLod_R{ReturnType}(sampled_image, ...);
-  void visitCallReadImageWithSampler(CallInst *CI, StringRef MangledName,
-                                     StringRef DemangledName);
+  void visitCallReadImageWithSampler(CallInst *CI, StringRef MangledName);
 
   /// Transform read_image with msaa image arguments.
   /// Sample argument must be acoded as Image Operand.

llvm-spirv/lib/SPIRV/SPIRVToOCL.cpp

@@ -751,43 +751,27 @@ void SPIRVToOCLBase::visitCallGroupWaitEvents(CallInst *CI, Op OC) {
       &Attrs);
 }
 
-static std::string getTypeSuffix(Type *T, bool IsSigned) {
-  std::string Suffix;
+static char getTypeSuffix(Type *T) {
+  char Suffix;
 
   Type *ST = T->getScalarType();
   if (ST->isHalfTy())
-    Suffix = "h";
+    Suffix = 'h';
   else if (ST->isFloatTy())
-    Suffix = "f";
-  else if (IsSigned)
-    Suffix = "i";
+    Suffix = 'f';
   else
-    Suffix = "ui";
+    Suffix = 'i';
 
   return Suffix;
 }
 
 void SPIRVToOCLBase::mutateArgsForImageOperands(std::vector<Value *> &Args,
-                                                unsigned ImOpArgIndex,
-                                                bool &IsSigned) {
-  // Default to signed.
-  IsSigned = true;
+                                                unsigned ImOpArgIndex) {
   if (Args.size() > ImOpArgIndex) {
     ConstantInt *ImOp = dyn_cast<ConstantInt>(Args[ImOpArgIndex]);
     uint64_t ImOpValue = 0;
     if (ImOp)
       ImOpValue = ImOp->getZExtValue();
-    unsigned SignZeroExtMasks = ImageOperandsMask::ImageOperandsSignExtendMask |
-                                ImageOperandsMask::ImageOperandsZeroExtendMask;
-    // If one of the SPIR-V 1.4 SignExtend/ZeroExtend operands is present, take
-    // it into account and drop the mask.
-    if (ImOpValue & SignZeroExtMasks) {
-      if (ImOpValue & ImageOperandsMask::ImageOperandsZeroExtendMask)
-        IsSigned = false;
-      ImOpValue &= ~SignZeroExtMasks;
-      Args[3] = getInt32(M, ImOpValue);
-      ImOp = cast<ConstantInt>(Args[3]);
-    }
     // Drop "Image Operands" argument.
     Args.erase(Args.begin() + ImOpArgIndex);
 
@@ -801,6 +785,7 @@ void SPIRVToOCLBase::mutateArgsForImageOperands(std::vector<Value *> &Args,
   }
 }
 
+// TODO: Handle unsigned integer return type. May need spec change.
 void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
                                                                  Op OC) {
   assert(CI->getCalledFunction() && "Unexpected indirect call");
@@ -822,8 +807,7 @@ void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
     auto Sampler = CallSampledImg->getArgOperand(1);
     Args[0] = Img;
     Args.insert(Args.begin() + 1, Sampler);
-    bool IsSigned;
-    mutateArgsForImageOperands(Args, 3, IsSigned);
+    mutateArgsForImageOperands(Args, 3);
     if (CallSampledImg->hasOneUse()) {
       CallSampledImg->replaceAllUsesWith(
           UndefValue::get(CallSampledImg->getType()));
@@ -834,8 +818,7 @@ void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
     if (auto VT = dyn_cast<VectorType>(T))
       T = VT->getElementType();
     RetTy = IsDepthImage ? T : CI->getType();
-    return std::string(kOCLBuiltinName::SampledReadImage) +
-           getTypeSuffix(T, IsSigned);
+    return std::string(kOCLBuiltinName::SampledReadImage) + getTypeSuffix(T);
   };
 
   auto ModifyRetTy = [=](CallInst *NewCI) -> Instruction * {
@@ -859,13 +842,11 @@ void SPIRVToOCLBase::visitCallSPIRVImageWriteBuiltIn(CallInst *CI, Op OC) {
       M, CI,
       [=](CallInst *, std::vector<Value *> &Args) {
         llvm::Type *T = Args[2]->getType();
-        bool IsSigned;
-        mutateArgsForImageOperands(Args, 3, IsSigned);
+        mutateArgsForImageOperands(Args, 3);
         if (Args.size() > 3) {
           std::swap(Args[2], Args[3]);
         }
-        return std::string(kOCLBuiltinName::WriteImage) +
-               getTypeSuffix(T, IsSigned);
+        return std::string(kOCLBuiltinName::WriteImage) + getTypeSuffix(T);
       },
       &Attrs);
 }
@@ -876,11 +857,9 @@ void SPIRVToOCLBase::visitCallSPIRVImageReadBuiltIn(CallInst *CI, Op OC) {
   mutateCallInstOCL(
       M, CI,
       [=](CallInst *, std::vector<Value *> &Args) {
-        bool IsSigned;
-        mutateArgsForImageOperands(Args, 2, IsSigned);
+        mutateArgsForImageOperands(Args, 2);
         llvm::Type *T = CI->getType();
-        return std::string(kOCLBuiltinName::ReadImage) +
-               getTypeSuffix(T, IsSigned);
+        return std::string(kOCLBuiltinName::ReadImage) + getTypeSuffix(T);
       },
       &Attrs);
 }

llvm-spirv/lib/SPIRV/SPIRVToOCL.h

@@ -273,10 +273,9 @@ class SPIRVToOCLBase : public InstVisitor<SPIRVToOCLBase> {
   std::string translateOpaqueType(StringRef STName);
 
   /// Mutate the argument list based on (optional) image operands at position
-  /// ImOpArgIndex.  Set IsSigned according to any SignExtend/ZeroExtend Image
-  /// Operands present in Args, or default to signed if there are none.
+  /// ImOpArgIndex.
   void mutateArgsForImageOperands(std::vector<Value *> &Args,
-                                  unsigned ImOpArgIndex, bool &IsSigned);
+                                  unsigned ImOpArgIndex);
 
 protected:
   Module *M;

llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.cpp

@@ -157,47 +157,6 @@ std::vector<SPIRVType *> SPIRVInstruction::getOperandTypes() {
   return getOperandTypes(getOperands());
 }
 
-size_t SPIRVImageInstBase::getImageOperandsIndex() const {
-  switch (OpCode) {
-  case OpImageRead:
-  case OpImageSampleExplicitLod:
-    return 2;
-  case OpImageWrite:
-    return 3;
-  default:
-    return ~0U;
-  }
-}
-
-void SPIRVImageInstBase::setOpWords(const std::vector<SPIRVWord> &OpsArg) {
-  std::vector<SPIRVWord> Ops = OpsArg;
-
-  // If the Image Operands field has the SignExtend or ZeroExtend bit set,
-  // either raise the minimum SPIR-V version to 1.4, or drop the operand
-  // if SPIR-V 1.4 cannot be emitted.
-  size_t ImgOpsIndex = getImageOperandsIndex();
-  if (ImgOpsIndex != ~0U && ImgOpsIndex < Ops.size()) {
-    SPIRVWord ImgOps = Ops[ImgOpsIndex];
-    unsigned SignZeroExtMasks = ImageOperandsMask::ImageOperandsSignExtendMask |
-                                ImageOperandsMask::ImageOperandsZeroExtendMask;
-    if (ImgOps & SignZeroExtMasks) {
-      SPIRVModule *M = getModule();
-      if (M->isAllowedToUseVersion(VersionNumber::SPIRV_1_4)) {
-        M->setMinSPIRVVersion(static_cast<SPIRVWord>(VersionNumber::SPIRV_1_4));
-      } else {
-        // Drop SignExtend/ZeroExtend if we cannot use SPIR-V 1.4.
-        Ops[ImgOpsIndex] &= ~SignZeroExtMasks;
-        if (Ops[ImgOpsIndex] == 0) {
-          // Drop the Image Operands if SignExtend/ZeroExtend was the only
-          // bit set.
-          Ops.pop_back();
-        }
-      }
-    }
-  }
-  SPIRVInstTemplateBase::setOpWords(Ops);
-}
-
 bool isSpecConstantOpAllowedOp(Op OC) {
   static SPIRVWord Table[] = {
       OpSConvert,

llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.h

@@ -2763,12 +2763,6 @@ class SPIRVImageInstBase : public SPIRVInstTemplateBase {
   SPIRVCapVec getRequiredCapability() const override {
     return getVec(CapabilityImageBasic);
   }
-
-protected:
-  void setOpWords(const std::vector<SPIRVWord> &OpsArg) override;
-
-private:
-  size_t getImageOperandsIndex() const;
 };
 
 #define _SPIRV_OP(x, ...)                                                      \

llvm-spirv/test/read_image.cl

@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple spir64 -fdeclare-opencl-builtins -finclude-default-header -O0 -cl-std=CL2.0 -emit-llvm-bc %s -o %t.bc -no-opaque-pointers
-// RUN: llvm-spirv --spirv-max-version=1.3 %t.bc -o %t.spv
+// RUN: llvm-spirv %t.bc -o %t.spv
 // RUN: spirv-val %t.spv
 // RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 // RUN: llvm-spirv -r %t.spv -o %t.rev.bc --spirv-target-env=SPV-IR
 // RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-SPV-LLVM
-// RUN: llvm-spirv --spirv-max-version=1.3 %t.rev.bc -o %t.rev.spv
+// RUN: llvm-spirv %t.rev.bc -o %t.rev.spv
 // RUN: spirv-val %t.rev.spv
-// RUN: llvm-spirv --spirv-max-version=1.3 %t.rev.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+// RUN: llvm-spirv %t.rev.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
 
 // CHECK-SPIRV: TypeInt [[IntTy:[0-9]+]]
 // CHECK-SPIRV: TypeVector [[IVecTy:[0-9]+]] [[IntTy]]

llvm-spirv/test/transcoding/SampledImage.cl

@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple spir -cl-std=CL2.0 %s -fdeclare-opencl-builtins -finclude-default-header -emit-llvm-bc -o %t.bc -no-opaque-pointers
 // RUN: llvm-spirv %t.bc -spirv-text -o %t.txt
 // RUN: FileCheck < %t.txt %s --check-prefix=CHECK-SPIRV
-// RUN: llvm-spirv --spirv-max-version=1.3 %t.bc -o %t.spv
+// RUN: llvm-spirv %t.bc -o %t.spv
 // RUN: spirv-val %t.spv
 // RUN: llvm-spirv -r %t.spv -o %t.rev.bc
 // RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-LLVM

llvm-spirv/test/transcoding/image_signedness.ll

@@ -1,5 +1,8 @@
 ; Test that signedness of calls to read_image(u)i/write_image(u)i is preserved.
 
+; TODO: Translator does not handle signedness for read_image/write_image yet.
+; XFAIL: *
+
 ; RUN: llvm-as %s -o %t.bc
 ; RUN: llvm-spirv %t.bc -o %t.spv
 ; RUN: spirv-val %t.spv