RValue references no more! (#2160)

In a few places, we used rvalue references as if they meant read-only references, which is not at all their semantics. This change ensures that we never deal with rvalue references, which in any case have no place in HLSL and only introduce weird edge cases, but only with const and non-const lvalue references.

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -3063,7 +3063,7 @@ private:
     QualType sampleSliceType = m_context->getRecordType(sampleSliceTypeDecl);
 
     CXXMethodDecl* sampleSubscriptDecl = CreateObjectFunctionDeclarationWithParams(*m_context,
-      sampleTypeDecl, m_context->getRValueReferenceType(sampleSliceType), // TODO: choose LValueRef if writable.
+      sampleTypeDecl, m_context->getLValueReferenceType(sampleSliceType),
       ArrayRef<QualType>(indexer0Type), ArrayRef<StringRef>(StringRef(indexer0Name)), subscriptName, true);
     sampleTypeDecl->completeDefinition();
 
@@ -3108,10 +3108,8 @@ private:
         typeDecl->getTemplateParameters()->getParam(0));
     QualType resultType = m_context->getTemplateTypeParmType(
         templateDepth, 0, ParameterPackFalse, templateTypeParmDecl);
-    if (isReadWrite)
-      resultType = m_context->getLValueReferenceType(resultType, false);
-    else
-      resultType = m_context->getRValueReferenceType(resultType);
+    if (!isReadWrite) resultType = m_context->getConstType(resultType);
+    resultType = m_context->getLValueReferenceType(resultType);
 
     QualType indexType =
         op.SubscriptCardinality == 1
@@ -4638,10 +4636,10 @@ public:
       return SpecFunc;
     }
 
-    // Change return type to rvalue reference type for aggregate types
+    // Change return type to lvalue reference type for aggregate types
     QualType retTy = parameterTypes[0];
     if (hlsl::IsHLSLAggregateType(retTy))
-      parameterTypes[0] = m_context->getRValueReferenceType(retTy);
+      parameterTypes[0] = m_context->getLValueReferenceType(retTy);
 
     // Create a new specialization.
     SmallVector<ParameterModifier, g_MaxIntrinsicParamCount> paramMods;
@@ -4950,13 +4948,8 @@ FunctionDecl* HLSLExternalSource::AddSubscriptSpecialization(
   // Create the template argument.
   bool isReadWrite = GetBasicKindProps(findResult.Kind) & BPROP_RWBUFFER;
   QualType resultType = objectElement;
-  if (isReadWrite)
-    resultType = m_context->getLValueReferenceType(resultType, false);
-  else {
-    // Add const to avoid write.
-    resultType = m_context->getConstType(resultType);
-    resultType = m_context->getLValueReferenceType(resultType);
-  }
+  if (!isReadWrite) resultType = m_context->getConstType(resultType);
+  resultType = m_context->getLValueReferenceType(resultType);
 
   TemplateArgument templateArgument(resultType);
   unsigned subscriptCardinality =

tools/clang/test/CodeGenHLSL/batch/expressions/argument_passing/xvalue_to_rvalue_regression.hlsl

@@ -0,0 +1,14 @@
+// RUN: %dxc -E main -T vs_6_0 %s | FileCheck %s
+
+// Regression test for a crash because some intrinsics like StructuredBuffer::Load()
+// used to return rvalue references, which classified as an xvalue by clang
+// and misbehaved when used as an argument to a function expecting a prvalue.
+
+// CHECK: call %dx.types.ResRet.i32 @dx.op.bufferLoad.i32
+// CHECK: extractvalue %dx.types.ResRet.i32
+// CHECK: call void @dx.op.bufferStore.i32
+
+struct S { int x; };
+StructuredBuffer<S> structbuf;
+AppendStructuredBuffer<S> appbuf;
+void main() { appbuf.Append(structbuf.Load(0)); }

tools/clang/test/CodeGenSPIRV/type.append.consume-structured-buffer.cast.hlsl

@@ -60,6 +60,8 @@ void main() {
 // CHECK:       [[p_5:%\d+]] = OpAccessChain %_ptr_Uniform_struct_with_bool %consume_struct_with_bool %uint_0 {{%\d+}}
 // CHECK-NEXT:  [[p_6:%\d+]] = OpAccessChain %_ptr_Uniform_uint [[p_5]] %int_3
 // CHECK-NEXT:  [[i_5:%\d+]] = OpLoad %uint [[p_6]]
+// CHECK-NEXT: [[bi_5:%\d+]] = OpINotEqual %bool [[i_5]] %uint_0
+// CHECK-NEXT:  [[i_5:%\d+]] = OpSelect %uint [[bi_5]] %uint_1 %uint_0
 // CHECK-NEXT:                 OpStore [[p_4]] [[i_5]]
   append_bool.Append(consume_struct_with_bool.Consume().elem_bool);
 
@@ -146,6 +148,8 @@ void main() {
 // CHECK:       [[p_19:%\d+]] = OpAccessChain %_ptr_Uniform_struct_with_bool %consume_struct_with_bool %uint_0 {{%\d+}}
 // CHECK-NEXT:  [[p_20:%\d+]] = OpAccessChain %_ptr_Uniform_v2uint [[p_19]] %int_0
 // CHECK-NEXT: [[vu_20:%\d+]] = OpLoad %v2uint [[p_20]]
+// CHECK-NEXT: [[vb_20:%\d+]] = OpINotEqual %v2bool [[vu_20]] {{%\d+}}
+// CHECK-NEXT: [[vu_20:%\d+]] = OpSelect %v2uint [[vb_20]] {{%\d+}} {{%\d+}}
 // CHECK-NEXT:                  OpStore {{%\d+}} [[vu_20]]
   append_v2bool.Append(consume_struct_with_bool.Consume().elem_v2bool);
 
@@ -259,6 +263,8 @@ void main() {
 
 // CHECK:       [[p_41:%\d+]] = OpAccessChain %_ptr_Uniform_uint {{%\d+}} %int_0
 // CHECK-NEXT:  [[i_41:%\d+]] = OpLoad %uint [[p_41]]
+// CHECK-NEXT:  [[b_41:%\d+]] = OpINotEqual %bool [[i_41]] %uint_0
+// CHECK-NEXT:  [[i_41:%\d+]] = OpSelect %uint [[b_41]] %uint_1 %uint_0
 // CHECK-NEXT:                  OpStore {{%\d+}} [[i_41]]
   append_bool.Append(consume_struct_with_bool.Consume().elem_v2bool.x);