From f7ffe9252802976e72e8f2add07a1aeedfdff515 Mon Sep 17 00:00:00 2001 From: Tristan Labelle Date: Fri, 3 May 2019 09:12:29 -0700 Subject: [PATCH] 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 | 21 +++++++------------ .../xvalue_to_rvalue_regression.hlsl | 14 +++++++++++++ ...append.consume-structured-buffer.cast.hlsl | 6 ++++++ 3 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 tools/clang/test/CodeGenHLSL/batch/expressions/argument_passing/xvalue_to_rvalue_regression.hlsl diff --git a/tools/clang/lib/Sema/SemaHLSL.cpp b/tools/clang/lib/Sema/SemaHLSL.cpp index d43859415..b682e487b 100644 --- a/tools/clang/lib/Sema/SemaHLSL.cpp +++ b/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(indexer0Type), ArrayRef(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 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 = diff --git a/tools/clang/test/CodeGenHLSL/batch/expressions/argument_passing/xvalue_to_rvalue_regression.hlsl b/tools/clang/test/CodeGenHLSL/batch/expressions/argument_passing/xvalue_to_rvalue_regression.hlsl new file mode 100644 index 000000000..755966203 --- /dev/null +++ b/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 structbuf; +AppendStructuredBuffer appbuf; +void main() { appbuf.Append(structbuf.Load(0)); } \ No newline at end of file diff --git a/tools/clang/test/CodeGenSPIRV/type.append.consume-structured-buffer.cast.hlsl b/tools/clang/test/CodeGenSPIRV/type.append.consume-structured-buffer.cast.hlsl index fa816f2b9..5429a145c 100644 --- a/tools/clang/test/CodeGenSPIRV/type.append.consume-structured-buffer.cast.hlsl +++ b/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);