[PATCH] D125324: AMDGPU/GISel: Factor out AMDGPURegisterBankInfo::buildReadFirstLane
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 16 14:52:09 PDT 2022
arsenm accepted this revision.
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:719
+
+ const TargetRegisterClass *Constrained =
+ constrainGenericRegister(SrcPart, AMDGPU::VGPR_32RegClass, MRI);
----------------
kosarev wrote:
> Putting this under `#ifndef NDEBUG` would prevent calling `constrainGenericRegister()` in no-asserts builds.
Why would you want to do that? The operands always need to be constrained
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1017
MachineIRBuilder B(MI);
+ B.setDebugLoc(MI.getDebugLoc());
----------------
Doesn't this happen in the constructor? (I do have a patch I Need to pick up to stop reconstructing this in RegBankSelect)
================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/image-waterfall-loop-O0.ll:52
-; CHECK-NEXT: buffer_store_dword v9, off, s[0:3], s32 offset:36 ; 4-byte Folded Spill
-; CHECK-NEXT: v_mov_b32_e32 v9, v1
-; CHECK-NEXT: v_mov_b32_e32 v8, v0
----------------
This looks like an improvement, but you also seem to have ended up with more instructions inside the loop (I guess this is -O0 so it doesn't really matter either way)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125324/new/
https://reviews.llvm.org/D125324
More information about the llvm-commits
mailing list