[PATCH] D12112: x32. Fixes jmp %reg in x32
JF Bastien via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 18 14:49:43 PDT 2015
jfb added inline comments.
================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2281
@@ +2280,3 @@
+ case ISD::BRIND: {
+ if (Subtarget->isTarget64BitILP32() && !Subtarget->isTargetNaCl()) {
+ const SDValue &Target = Node->getOperand(1);
----------------
Could you comment on why not NaCl? Maybe restyle to be more readable:
```
case ISD::BRIND: {
if (Subtarget->isTargetNaCl())
// Not NaCl because...
break;
if (Subtarget->isTarget64BitILP32()) {
// ...
}
break;
```
================
Comment at: test/CodeGen/X86/x32-indirectbr.ll:7
@@ +6,3 @@
+; registers. Therefore, x32 CodeGen needs to zero extend indirectbr's target to
+; 64-bit.
+
----------------
It seems like `test2` relies on switch lowering's specific implementation? Is this tested here because it's generated through a different code path than `indirectbr`? I'd like to make sure that the tests isn't brittle if e.g. fast-isel changes, while still making sure you have proper test coverage.
================
Comment at: test/CodeGen/X86/x32-indirectbr.ll:9
@@ +8,3 @@
+
+define i8 @test1() nounwind ssp {
+entry:
----------------
`CHECK-LABEL`
================
Comment at: test/CodeGen/X86/x32-indirectbr.ll:22
@@ +21,3 @@
+; CHECK: movl {{.*}}, %{{e|r}}[[REG:.[^d]*]]{{d?}}
+; CHECK: jmpq *%r[[REG]]
+
----------------
`CHECK-NEXT` for this line.
================
Comment at: test/CodeGen/X86/x32-indirectbr.ll:24
@@ +23,3 @@
+
+define i32 @test2(i32 %idx) nounwind ssp {
+entry:
----------------
`CHECK-LABEL`
================
Comment at: test/CodeGen/X86/x32-indirectbr.ll:56
@@ +55,2 @@
+; CHECK: movl {{.*}}, %{{e|r}}[[REG:.[^d]*]]{{d?}}
+; CHECK: jmpq *%r[[REG]]
----------------
`CHECK-NEXT`
http://reviews.llvm.org/D12112
More information about the llvm-commits
mailing list