Skip to content

Commit f0551ea

Browse files
committed
code review feedback - add comment, simplify arg handling for better readability
1 parent 516bd63 commit f0551ea

File tree

3 files changed

+20
-30
lines changed

3 files changed

+20
-30
lines changed

clang/lib/CodeGen/CGHLSLBuiltins.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -302,16 +302,11 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID,
302302

303303
auto [IntrinsicID, HasNameArg] =
304304
CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
305-
if (HasNameArg) {
306-
Value *NameOp = EmitScalarExpr(E->getArg(5));
307-
return Builder.CreateIntrinsic(HandleTy, IntrinsicID,
308-
ArrayRef<Value *>{SpaceOp, RegisterOp,
309-
RangeOp, IndexOp,
310-
NonUniform, NameOp});
311-
}
312-
return Builder.CreateIntrinsic(
313-
HandleTy, IntrinsicID,
314-
ArrayRef<Value *>{SpaceOp, RegisterOp, RangeOp, IndexOp, NonUniform});
305+
SmallVector<Value *> Args{SpaceOp, RegisterOp, RangeOp, IndexOp,
306+
NonUniform};
307+
if (HasNameArg)
308+
Args.push_back(EmitScalarExpr(E->getArg(5)));
309+
return Builder.CreateIntrinsic(HandleTy, IntrinsicID, Args);
315310
}
316311
case Builtin::BI__builtin_hlsl_resource_handlefromimplicitbinding: {
317312
llvm::Type *HandleTy = CGM.getTypes().ConvertType(E->getType());
@@ -326,16 +321,10 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID,
326321

327322
auto [IntrinsicID, HasNameArg] =
328323
CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
329-
if (HasNameArg) {
330-
Value *NameOp = EmitScalarExpr(E->getArg(5));
331-
return Builder.CreateIntrinsic(HandleTy, IntrinsicID,
332-
ArrayRef<Value *>{OrderID, SpaceOp,
333-
RangeOp, IndexOp,
334-
NonUniform, NameOp});
335-
}
336-
return Builder.CreateIntrinsic(
337-
HandleTy, IntrinsicID,
338-
ArrayRef<Value *>{OrderID, SpaceOp, RangeOp, IndexOp, NonUniform});
324+
SmallVector<Value *> Args{OrderID, SpaceOp, RangeOp, IndexOp, NonUniform};
325+
if (HasNameArg)
326+
Args.push_back(EmitScalarExpr(E->getArg(5)));
327+
return Builder.CreateIntrinsic(HandleTy, IntrinsicID, Args);
339328
}
340329
case Builtin::BI__builtin_hlsl_all: {
341330
Value *Op0 = EmitScalarExpr(E->getArg(0));

clang/lib/CodeGen/CGHLSLRuntime.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -608,22 +608,18 @@ void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
608608
// buffer with explicit binding
609609
if (RBA->hasRegisterSlot()) {
610610
auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
611+
SmallVector<Value *> Args{Space, RegSlot, RangeSize, Index, NonUniform};
611612
if (Name)
612-
initializeBuffer(CGM, GV, IntrinsicID,
613-
{Space, RegSlot, RangeSize, Index, NonUniform, Name});
614-
else
615-
initializeBuffer(CGM, GV, IntrinsicID,
616-
{Space, RegSlot, RangeSize, Index, NonUniform});
613+
Args.push_back(Name);
614+
initializeBuffer(CGM, GV, IntrinsicID, Args);
617615
} else {
618616
// buffer with implicit binding
619617
auto *OrderID =
620618
llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
619+
SmallVector<Value *> Args{OrderID, Space, RangeSize, Index, NonUniform};
621620
if (Name)
622-
initializeBuffer(CGM, GV, IntrinsicID,
623-
{OrderID, Space, RangeSize, Index, NonUniform, Name});
624-
else
625-
initializeBuffer(CGM, GV, IntrinsicID,
626-
{OrderID, Space, RangeSize, Index, NonUniform});
621+
Args.push_back(Name);
622+
initializeBuffer(CGM, GV, IntrinsicID, Args);
627623
}
628624
}
629625

clang/lib/CodeGen/CGHLSLRuntime.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,12 @@ class CGHLSLRuntime {
125125
// End of reserved area for HLSL intrinsic getters.
126126
//===----------------------------------------------------------------------===//
127127

128+
// Returns ID of the intrinsic that initializes resource handle from binding
129+
// and a bool value indicating whether the last argument of the intrinsic is
130+
// the resource name (not all targets need that).
128131
std::pair<llvm::Intrinsic::ID, bool> getCreateHandleFromBindingIntrinsic();
132+
133+
// Same as above but for implicit binding.
129134
std::pair<llvm::Intrinsic::ID, bool>
130135
getCreateHandleFromImplicitBindingIntrinsic();
131136

0 commit comments

Comments
 (0)