Skip to content

Conversation

kosarev
Copy link
Collaborator

@kosarev kosarev commented Aug 14, 2024

And declare it to take an MCRegister.

Also rename related entities and remove a comment for the function that depending on its purpose is either irrelevant or misleading.

And declare it to take an MCRegister.

Also rename related entities and remove a comment for the function
that depending on its purpose is either irrelevant or misleading.
@llvmbot
Copy link
Member

llvmbot commented Aug 14, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Ivan Kosarev (kosarev)

Changes

And declare it to take an MCRegister.

Also rename related entities and remove a comment for the function that depending on its purpose is either irrelevant or misleading.


Full diff: https://github.com/llvm/llvm-project/pull/103888.diff

8 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp (+3-3)
  • (modified) llvm/lib/Target/AMDGPU/SIDefines.h (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.td (+3-3)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp (+2-2)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h (+1-2)
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 725f49984483c1..1a10206eea2374 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8546,7 +8546,7 @@ static void cvtVOP3DstOpSelOnly(MCInst &Inst, const MCRegisterInfo &MRI) {
   uint32_t ModVal = Inst.getOperand(ModIdx).getImm();
   if (DstOp.isReg() &&
       MRI.getRegClass(AMDGPU::VGPR_16RegClassID).contains(DstOp.getReg())) {
-    if (AMDGPU::isHi(DstOp.getReg(), MRI))
+    if (AMDGPU::isHi16Reg(DstOp.getReg(), MRI))
       ModVal |= SISrcMods::DST_OP_SEL;
   } else {
     if ((OpSel & (1 << SrcNum)) != 0)
@@ -8826,7 +8826,7 @@ void AMDGPUAsmParser::cvtVOP3P(MCInst &Inst, const OperandVector &Operands,
     if (SrcOp.isReg() && getMRI()
                              ->getRegClass(AMDGPU::VGPR_16RegClassID)
                              .contains(SrcOp.getReg())) {
-      bool VGPRSuffixIsHi = AMDGPU::isHi(SrcOp.getReg(), *getMRI());
+      bool VGPRSuffixIsHi = AMDGPU::isHi16Reg(SrcOp.getReg(), *getMRI());
       if (VGPRSuffixIsHi)
         ModVal |= SISrcMods::OP_SEL_0;
     } else {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
index 2c9d17d448eadd..2af1f919730257 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
@@ -608,7 +608,7 @@ void AMDGPUMCCodeEmitter::getMachineOpValueT16(
         AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::vdst);
     if (VDstMOIdx != -1) {
       auto DstReg = MI.getOperand(VDstMOIdx).getReg();
-      if (AMDGPU::isHi(DstReg, MRI))
+      if (AMDGPU::isHi16Reg(DstReg, MRI))
         Op |= SISrcMods::DST_OP_SEL;
     }
   } else if ((int)OpNo == AMDGPU::getNamedOperandIdx(
@@ -626,7 +626,7 @@ void AMDGPUMCCodeEmitter::getMachineOpValueT16(
   auto SrcReg = SrcMO.getReg();
   if (AMDGPU::isSGPR(SrcReg, &MRI))
     return;
-  if (AMDGPU::isHi(SrcReg, MRI))
+  if (AMDGPU::isHi16Reg(SrcReg, MRI))
     Op |= SISrcMods::OP_SEL_0;
 }
 
@@ -637,7 +637,7 @@ void AMDGPUMCCodeEmitter::getMachineOpValueT16Lo128(
   if (MO.isReg()) {
     uint16_t Encoding = MRI.getEncodingValue(MO.getReg());
     unsigned RegIdx = Encoding & AMDGPU::HWEncoding::REG_IDX_MASK;
-    bool IsHi = Encoding & AMDGPU::HWEncoding::IS_HI;
+    bool IsHi = Encoding & AMDGPU::HWEncoding::IS_HI16;
     bool IsVGPR = Encoding & AMDGPU::HWEncoding::IS_VGPR;
     assert((!IsVGPR || isUInt<7>(RegIdx)) && "VGPR0-VGPR127 expected!");
     Op = (IsVGPR ? 0x100 : 0) | (IsHi ? 0x80 : 0) | RegIdx;
diff --git a/llvm/lib/Target/AMDGPU/SIDefines.h b/llvm/lib/Target/AMDGPU/SIDefines.h
index 2f001db776975f..fb3d83ca30d198 100644
--- a/llvm/lib/Target/AMDGPU/SIDefines.h
+++ b/llvm/lib/Target/AMDGPU/SIDefines.h
@@ -371,7 +371,7 @@ enum : unsigned {
   REG_IDX_MASK = 0xff,
   IS_VGPR = 1 << 8,
   IS_AGPR = 1 << 9,
-  IS_HI = 1 << 10, // High 16-bit register.
+  IS_HI16 = 1 << 10,
 };
 } // namespace HWEncoding
 
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 8af5c364509f0e..9147242046ceda 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -954,8 +954,8 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
     bool IsSGPRSrc = AMDGPU::SReg_LO16RegClass.contains(SrcReg);
     bool IsAGPRDst = AMDGPU::AGPR_LO16RegClass.contains(DestReg);
     bool IsAGPRSrc = AMDGPU::AGPR_LO16RegClass.contains(SrcReg);
-    bool DstLow = !AMDGPU::isHi(DestReg, RI);
-    bool SrcLow = !AMDGPU::isHi(SrcReg, RI);
+    bool DstLow = !AMDGPU::isHi16Reg(DestReg, RI);
+    bool SrcLow = !AMDGPU::isHi16Reg(SrcReg, RI);
     MCRegister NewDestReg = RI.get32BitRegister(DestReg);
     MCRegister NewSrcReg = RI.get32BitRegister(SrcReg);
 
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
index ee72837a50fc43..7523b619748cc7 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
@@ -332,7 +332,7 @@ SIRegisterInfo::SIRegisterInfo(const GCNSubtarget &ST)
   RegPressureIgnoredUnits.resize(getNumRegUnits());
   RegPressureIgnoredUnits.set(*regunits(MCRegister::from(AMDGPU::M0)).begin());
   for (auto Reg : AMDGPU::VGPR_16RegClass) {
-    if (AMDGPU::isHi(Reg, *this))
+    if (AMDGPU::isHi16Reg(Reg, *this))
       RegPressureIgnoredUnits.set(*regunits(Reg).begin());
   }
 
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
index 519048356f764d..d3e39464fea396 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
@@ -123,7 +123,7 @@ class SIRegisterTuples<list<SubRegIndex> Indices, RegisterClass RC,
 //  Declarations that describe the SI registers
 //===----------------------------------------------------------------------===//
 class SIReg <string n, bits<8> regIdx = 0, bit isVGPR = 0,
-             bit isAGPR = 0, bit isHi = 0> : Register<n> {
+             bit isAGPR = 0, bit isHi16 = 0> : Register<n> {
   let Namespace = "AMDGPU";
 
   // These are generic helper values we use to form actual register
@@ -132,7 +132,7 @@ class SIReg <string n, bits<8> regIdx = 0, bit isVGPR = 0,
   let HWEncoding{7-0} = regIdx;
   let HWEncoding{8} = isVGPR;
   let HWEncoding{9} = isAGPR;
-  let HWEncoding{10} = isHi;
+  let HWEncoding{10} = isHi16;
 
   int Index = !cast<int>(regIdx);
 }
@@ -161,7 +161,7 @@ multiclass SIRegLoHi16 <string n, bits<8> regIdx, bit ArtificialHigh = 1,
                         bit isVGPR = 0, bit isAGPR = 0> {
   def _LO16 : SIReg<n#".l", regIdx, isVGPR, isAGPR>;
   def _HI16 : SIReg<!if(ArtificialHigh, "", n#".h"), regIdx, isVGPR, isAGPR,
-                    /* isHi */ 1> {
+                    /* isHi16 */ 1> {
     let isArtificial = ArtificialHigh;
   }
   def "" : RegisterWithSubRegs<n, [!cast<Register>(NAME#"_LO16"),
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 96d4863e94014c..0ca6266cc678b6 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -2237,8 +2237,8 @@ bool isSGPR(unsigned Reg, const MCRegisterInfo* TRI) {
     Reg == AMDGPU::SCC;
 }
 
-bool isHi(unsigned Reg, const MCRegisterInfo &MRI) {
-  return MRI.getEncodingValue(Reg) & AMDGPU::HWEncoding::IS_HI;
+bool isHi16Reg(MCRegister Reg, const MCRegisterInfo &MRI) {
+  return MRI.getEncodingValue(Reg) & AMDGPU::HWEncoding::IS_HI16;
 }
 
 #define MAP_REG2REG \
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index 429c3ad335d213..a4e6a7ebe0558b 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -1314,8 +1314,7 @@ bool hasSMRDSignedImmOffset(const MCSubtargetInfo &ST);
 bool isSGPR(unsigned Reg, const MCRegisterInfo* TRI);
 
 /// \returns if \p Reg occupies the high 16-bits of a 32-bit register.
-/// The bit indicating isHi is the LSB of the encoding.
-bool isHi(unsigned Reg, const MCRegisterInfo &MRI);
+bool isHi16Reg(MCRegister Reg, const MCRegisterInfo &MRI);
 
 /// If \p Reg is a pseudo reg, return the correct hardware register given
 /// \p STI otherwise return \p Reg.

@kosarev
Copy link
Collaborator Author

kosarev commented Aug 14, 2024

@broxigarchen Guo, also tagging you for visibility.

Copy link
Contributor

@Sisyph Sisyph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kosarev kosarev merged commit f0fe6c6 into llvm:main Aug 14, 2024
@kosarev kosarev deleted the ishi branch August 14, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants