Page MenuHomePhabricator

Use PC-relative address for x32 TLS address
Needs ReviewPublic

Authored by hjl.tools on Jan 22 2016, 10:45 AM.

Details

Summary

Since x32 supports PC-relative address, it shouldn't use EBX for TLS
address. Instead of checking N.getValueType(), we should check
Subtarget->is32Bit(). This fixes PR 22676.

The updated diff with more context.

Diff Detail

Event Timeline

hjl.tools updated this revision to Diff 45704.Jan 22 2016, 10:45 AM
hjl.tools retitled this revision from to Use PC-relative address for x32 TLS address .
hjl.tools updated this object.
hjl.tools added reviewers: zansari, DavidKreitzer.
hjl.tools added a subscriber: llvm-commits.
mgrang added a subscriber: mgrang.Jan 22 2016, 11:32 AM

Could you please upload your patch with more context? (Preferably the full src file).

lib/Target/X86/X86ISelDAGToDAG.cpp
1682

Is there any specific reason why the if-else clauses have been reversed? i.e: why are we checking for 64 bit first? Can we still do this:

if (Subtarget->is32Bit()) {

  AM.Scale = 1;
  AM.IndexReg = CurDAG->getRegister(X86::EBX, MVT::i32);
}

else {

AM.IndexReg = CurDAG->getRegister(0, N.getValueType());

}

hjl.tools updated this revision to Diff 45727.Jan 22 2016, 12:27 PM
hjl.tools updated this object.

Use Subtarget->is32Bit()) instead. The full function now is:

/// This is only run on TargetGlobalTLSAddress nodes.
bool X86DAGToDAGISel::selectTLSADDRAddr(SDValue N, SDValue &Base,

                                      SDValue &Scale, SDValue &Index,
                                      SDValue &Disp, SDValue &Segment) {
assert(N.getOpcode() == ISD::TargetGlobalTLSAddress);
const GlobalAddressSDNode *GA = cast<GlobalAddressSDNode>(N);

X86ISelAddressMode AM;
AM.GV = GA->getGlobal();
AM.Disp += GA->getOffset();
AM.Base_Reg = CurDAG->getRegister(0, N.getValueType());
AM.SymbolFlags = GA->getTargetFlags();

if (Subtarget->is32Bit()) {
  AM.Scale = 1;
  AM.IndexReg = CurDAG->getRegister(X86::EBX, MVT::i32);
} else {
  AM.IndexReg = CurDAG->getRegister(0, N.getValueType());
}

getAddressOperands(AM, SDLoc(N), Base, Scale, Index, Disp, Segment);
return true;

}

This comment was removed by mgrang.

Since llvm doesn't support TLS for x32:

[hjl@gnu-mic-2 X86]$ cat x32-tls-1.ll
@x = thread_local global i32 0, align 4

define i32* @get_x() {
entry:

ret i32* @x

}
[hjl@gnu-mic-2 X86]$ /export/build/gnu/llvm-clang/build-x86_64-linux/bin/llc -relocation-model=pic -mtriple=x86_64-linux-gnux32 < x32-tls-1.ll
.text
.file "<stdin>"
LLVM ERROR: Cannot select: 0x29507a0: ch,glue = X86ISD::TLSADDR 0x28fc140, TargetGlobalTLSAddress:i32<i32* @x> 0 [TF=7]

0x2950670: i32 = TargetGlobalTLSAddress<i32* @x> 0 [TF=7]

In function: get_x
[hjl@gnu-mic-2 X86]$

I don't know how to create a test for this change.

hjl.tools updated this revision to Diff 56457.May 6 2016, 1:55 PM
hjl.tools updated this object.
hjl.tools added a subscriber: qcolombet.

If https://bugs.llvm.org/show_bug.cgi?id=38932 is indeed a duplicate of the bug filed for this, https://bugs.llvm.org/show_bug.cgi?id=22676 then it seems the filer of the duplicate has provided a simple test case.

I think the bug also popped up at https://bugs.llvm.org/show_bug.cgi?id=26472 judging by the function.