[PATCH] D137542: [AMDGPU] Use aperture registers instead of S_GETREG

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:119
 
+static bool IsApertureRegister64(Register Reg) {
+  if (!Reg.isPhysical())
----------------
Lowercase is


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:149-150
+  // register with a reference to its artificial HI 32 bits.
+  if (IsApertureRegister64(SrcReg))
+    return RBI.constrainGenericRegister(DstReg, AMDGPU::SGPR_64RegClass, *MRI);
+
----------------
You shouldn't need to special case this (also note there's no equivalent for the DAG). Either it needs to belong to a different class from SReg_64, or you need to reserve the high bits 


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5521-5524
+    return DAG.getNode(
+        ISD::TRUNCATE, DL, MVT::i32,
+        DAG.getNode(ISD::SRL, DL, MVT::i64,
+                    {SDValue(Mov, 0), DAG.getConstant(32, DL, MVT::i64)}));
----------------
This will get combined later to bitcast and extract later, but I guess that's more annoying to emit


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-copy.mir:485-488
+    %0:sreg_64(s64) = COPY $src_private_base
+    %1:sreg_64(s64) = COPY $src_private_limit
+    %2:sreg_64(s64) = COPY $src_shared_base
+    %3:sreg_64(s64) = COPY $src_shared_limit
----------------
A basic selection test shouldn't have pre-set register classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137542