[spirv] Provide comments regarding aliasing work.

tools/clang/include/clang/SPIRV/SpirvFunction.h

@@ -72,12 +72,6 @@ public:
   void setSourceLocation(SourceLocation loc) { functionLoc = loc; }
   SourceLocation getSourceLocation() const { return functionLoc; }
 
-  void setConstainsAliasComponent(bool isAlias) { containsAlias = isAlias; }
-  bool constainsAliasComponent() { return containsAlias; }
-
-  void setRValue() { rvalue = true; }
-  bool isRValue() { return rvalue; }
-
   void setFunctionName(llvm::StringRef name) { functionName = name; }
   llvm::StringRef getFunctionName() const { return functionName; }
 
@@ -85,6 +79,16 @@ public:
   void addVariable(SpirvVariable *);
   void addBasicBlock(SpirvBasicBlock *);
 
+  /// Legalization-specific code
+  ///
+  /// Note: the following methods are used for properly handling aliasing.
+  ///
+  /// TODO: Clean up aliasing and try to move it to a separate pass.
+  void setConstainsAliasComponent(bool isAlias) { containsAlias = isAlias; }
+  bool constainsAliasComponent() { return containsAlias; }
+  void setRValue() { rvalue = true; }
+  bool isRValue() { return rvalue; }
+
 private:
   uint32_t functionId; ///< This function's <result-id>
 
@@ -95,6 +99,12 @@ private:
   SpirvType *fnType; ///< The SPIR-V function type
   uint32_t fnTypeId; ///< result-id for the SPIR-V function type
 
+  /// Legalization-specific code
+  ///
+  /// Note: the following two member variables are currently needed in order to
+  /// support aliasing for functions.
+  ///
+  /// TODO: Clean up aliasing and try to move it to a separate pass.
   bool containsAlias; ///< Whether function return type is aliased
   bool rvalue;        ///< Whether the return value is an rvalue
 

tools/clang/include/clang/SPIRV/SpirvInstruction.h

@@ -149,9 +149,6 @@ public:
   SpirvLayoutRule getLayoutRule() const { return layoutRule; }
   void setLayoutRule(SpirvLayoutRule rule) { layoutRule = rule; }
 
-  void setContainsAliasComponent(bool contains) { containsAlias = contains; }
-  bool containsAliasComponent() const { return containsAlias; }
-
   void setStorageClass(spv::StorageClass sc) { storageClass = sc; }
   spv::StorageClass getStorageClass() const { return storageClass; }
 
@@ -164,6 +161,15 @@ public:
   void setNonUniform(bool nu = true) { isNonUniform_ = nu; }
   bool isNonUniform() const { return isNonUniform_; }
 
+  /// Legalization-specific code
+  ///
+  /// Note: the following two functions are currently needed in order to support
+  /// aliasing.
+  ///
+  /// TODO: Clean up aliasing and try to move it to a separate pass.
+  void setContainsAliasComponent(bool contains) { containsAlias = contains; }
+  bool containsAliasComponent() const { return containsAlias; }
+
 protected:
   // Forbid creating SpirvInstruction directly
   SpirvInstruction(Kind kind, spv::Op opcode, QualType astResultType,
@@ -1728,6 +1734,8 @@ private:
   uint32_t arrayMember;
 };
 
+// TODO: It'd be better to leave debug info attached to whatever entity they are
+// annotating and have a pass write out the binaries for them directly.
 class SpirvLineInfo : public SpirvInstruction {
 public:
   SpirvLineInfo(SpirvString *srcFile, uint32_t srcLine, uint32_t srcCol);

tools/clang/lib/SPIRV/EmitVisitor.cpp

@@ -1476,7 +1476,7 @@ uint32_t EmitTypeHandler::emitType(const SpirvType *type) {
   // Therefore, if we find a hybrid type when going through the emitting pass,
   // that is clearly a bug.
   else if (const auto *hybridType = dyn_cast<HybridType>(type)) {
-    assert(false && "found hybrid type when emitting SPIR-V");
+    llvm_unreachable("found hybrid type when emitting SPIR-V");
   }
   // Unhandled types
   else {

tools/clang/lib/SPIRV/LiteralTypeVisitor.cpp

@@ -143,34 +143,41 @@ bool LiteralTypeVisitor::visit(SpirvBinaryOp *inst) {
   auto *operand1 = inst->getOperand1();
   auto *operand2 = inst->getOperand2();
 
-  // We should not modify operand2 type in these operations:
-  if (op == spv::Op::OpShiftRightLogical ||
-      op == spv::Op::OpShiftRightArithmetic ||
-      op == spv::Op::OpShiftLeftLogical) {
+  switch (op) {
+  case spv::Op::OpShiftRightLogical:
+  case spv::Op::OpShiftRightArithmetic:
+  case spv::Op::OpShiftLeftLogical: {
     // Base (arg1) should have the same type as result type
     tryToUpdateInstLitType(inst->getOperand1(), resultType);
     // The shitf amount (arg2) cannot be a 64-bit type for a 32-bit base!
     tryToUpdateInstLitType(inst->getOperand2(), resultType);
     return true;
   }
-
   // The following operations have a boolean return type, so we cannot deduce
   // anything about the operand type from the result type. However, the two
   // operands in these operations must have the same bitwidth.
-  if (op == spv::Op::OpIEqual || op == spv::Op::OpINotEqual ||
-      op == spv::Op::OpUGreaterThan || op == spv::Op::OpSGreaterThan ||
-      op == spv::Op::OpUGreaterThanEqual ||
-      op == spv::Op::OpSGreaterThanEqual || op == spv::Op::OpULessThan ||
-      op == spv::Op::OpSLessThan || op == spv::Op::OpULessThanEqual ||
-      op == spv::Op::OpSLessThanEqual || op == spv::Op::OpFOrdEqual ||
-      op == spv::Op::OpFUnordEqual || op == spv::Op::OpFOrdNotEqual ||
-      op == spv::Op::OpFUnordNotEqual || op == spv::Op::OpFOrdLessThan ||
-      op == spv::Op::OpFUnordLessThan || op == spv::Op::OpFOrdGreaterThan ||
-      op == spv::Op::OpFUnordGreaterThan ||
-      op == spv::Op::OpFOrdLessThanEqual ||
-      op == spv::Op::OpFUnordLessThanEqual ||
-      op == spv::Op::OpFOrdGreaterThanEqual ||
-      op == spv::Op::OpFUnordGreaterThanEqual) {
+  case spv::Op::OpIEqual:
+  case spv::Op::OpINotEqual:
+  case spv::Op::OpUGreaterThan:
+  case spv::Op::OpSGreaterThan:
+  case spv::Op::OpUGreaterThanEqual:
+  case spv::Op::OpSGreaterThanEqual:
+  case spv::Op::OpULessThan:
+  case spv::Op::OpSLessThan:
+  case spv::Op::OpULessThanEqual:
+  case spv::Op::OpSLessThanEqual:
+  case spv::Op::OpFOrdEqual:
+  case spv::Op::OpFUnordEqual:
+  case spv::Op::OpFOrdNotEqual:
+  case spv::Op::OpFUnordNotEqual:
+  case spv::Op::OpFOrdLessThan:
+  case spv::Op::OpFUnordLessThan:
+  case spv::Op::OpFOrdGreaterThan:
+  case spv::Op::OpFUnordGreaterThan:
+  case spv::Op::OpFOrdLessThanEqual:
+  case spv::Op::OpFUnordLessThanEqual:
+  case spv::Op::OpFOrdGreaterThanEqual:
+  case spv::Op::OpFUnordGreaterThanEqual: {
     if (operand1->hasAstResultType() && operand2->hasAstResultType()) {
       const auto operand1Type = operand1->getAstResultType();
       const auto operand2Type = operand2->getAstResultType();
@@ -194,18 +201,22 @@ bool LiteralTypeVisitor::visit(SpirvBinaryOp *inst) {
         return true;
       }
     }
+    break;
   }
-
   // The result type of dot product is scalar but operands should be vector of
   // the same type.
-  if (op == spv::Op::OpDot) {
+  case spv::Op::OpDot: {
     tryToUpdateInstLitType(inst->getOperand1(),
                            inst->getOperand2()->getAstResultType());
     tryToUpdateInstLitType(inst->getOperand2(),
                            inst->getOperand1()->getAstResultType());
     return true;
   }
+  default:
+    break;
+  }
 
+  // General attempt to deduce operand types from the result type.
   tryToUpdateInstLitType(operand1, resultType);
   tryToUpdateInstLitType(operand2, resultType);
   return true;
@@ -429,4 +440,4 @@ bool LiteralTypeVisitor::visit(SpirvImageOp *inst) {
 }
 
 } // end namespace spirv
-} // namespace clang
+} // end namespace clang