[PATCH] D11604: AMDGPU/SI: Add support for 32-bit immediate SMRD offsets on SI

Matt Arsenault Matthew.Arsenault at amd.com
Wed Jul 29 11:50:08 PDT 2015


arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:1581
@@ +1580,3 @@
+
+  // FIXME: Support 20-bit offsets on VI.  We need to to pass subtaget
+  // information here.
----------------
Typo: subtaget

================
Comment at: lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:1587-1589
@@ +1586,5 @@
+bool AMDGPUOperand::isSMRDLiteralOffset() const {
+  // 32-bit literals are only supported on CI and we only want to use them
+  // when the offset is > 8-bits.
+  return isImm() && !isUInt<8>(getImm()) && isUInt<32>(getImm());
+}
----------------
I think you need to be careful here if the immediate is == 255. It is still is uint<8>, but the encoding value will end up reading the next 4 bytes of instruction instead of the right value.

================
Comment at: test/CodeGen/AMDGPU/smrd.ll:19
@@ -17,3 +18,3 @@
 ; GCN-LABEL: {{^}}smrd1:
-; SI: s_load_dword s{{[0-9]}}, s[{{[0-9]:[0-9]}}], 0xff ; encoding: [0xff
+; SICI: s_load_dword s{{[0-9]}}, s[{{[0-9]:[0-9]}}], 0xff ; encoding: [0xff
 ; VI: s_load_dword s{{[0-9]}}, s[{{[0-9]:[0-9]}}], 0x3fc
----------------
These should probably be split into SI and CI lines. I think on SI this will end up using a 32-bit encoding and will have to use on a 64-bit one on CI for 255

================
Comment at: test/MC/AMDGPU/smrd.s:11
@@ +10,3 @@
+
+s_load_dword s1, s[2:3], 0xfc
+// GCN: s_load_dword s1, s[2:3], 0xfc ; encoding: [0xfc,0x83,0x00,0xc0]
----------------
Should probably include a test with 0xff too


http://reviews.llvm.org/D11604







More information about the llvm-commits mailing list