[PATCH] D124450: [AMDGPU] Remove hasOneUse check from scalar select pattern

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 29 11:01:25 PDT 2022


arsenm added a comment.

In D124450#3475092 <https://reviews.llvm.org/D124450#3475092>, @arsenm wrote:

>> Shortly after this it crashes in `ScheduleDAGSDNodes::EmitPhysRegCopy` with: `MachineRegisterInfo.cpp:160: llvm::Register llvm::MachineRegisterInfo::createVirtualRegister(const llvm::TargetRegisterClass *, llvm::StringRef): Assertion 'RegClass->isAllocatable() && "Virtual register RegClass must be allocatable."' failed.`
>
> The should be able to use SReg_32 which we do handle



In D124450#3482523 <https://reviews.llvm.org/D124450#3482523>, @foad wrote:

>> The should be able to use SReg_32 which we do handle
>
> I can get that to work with a patch like this: https://reviews.llvm.org/differential/diff/426045/
>
> I'm not too happy that I had to change InstrEmitter::EmitCopyFromReg. It makes me wonder if we should be handling uniform compare+select patterns much more like a flags-based CPU does, either by gluing the s_cmp to the s_cselect, or using ISD::SELECT_CC instead of ISD::SELECT in the first place, so that it is all in one DAG node.

I was thinking we would add an SReg_32_PlusSCC class to use for for conditions (or I guess we could just add SCC directly to SReg_32). GlobalISel directly emits copies between SReg_32 and SCC but I guess InstrEmitter's magic requires an allocatable class


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124450/new/

https://reviews.llvm.org/D124450



More information about the llvm-commits mailing list