Page MenuHomePhabricator

[Sparc] allow tls_add/tls_call syntax in assembler parser
ClosedPublic

Authored by dcederman on Aug 8 2017, 6:10 AM.

Details

Summary

Removing unneeded isCodeGenOnly from tls-specific instructions - TLS_ADD/TLS_LD/TLS_LDX/TLS_CALL.

Diff Detail

Repository
rL LLVM

Event Timeline

fedor.sergeev created this revision.Aug 8 2017, 6:10 AM

This deficiency of asm parser was mentioned during https://reviews.llvm.org/D35567 review.

jyknight edited edge metadata.Oct 17 2017, 6:23 AM

LG, thanks!

jyknight accepted this revision.Oct 17 2017, 6:24 AM
This revision is now accepted and ready to land.Oct 17 2017, 6:24 AM

Running this sparc-tls-relocations.s test as part of a pre-commit routine it got an assert:
] bin/llvm-mc test/MC/Sparc/sparc-tls-relocations.s -arch=sparc -show-encoding
llvm-mc: /home/fsergeev/work/llvm-upstream/llvm/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp:169: unsigned int (anonymous namespace)::SparcMCCodeEmitter::getCallTargetOpValue(const llvm::MCInst &, unsigned int, SmallVectorImpl<llvm::MCFixup> &, const llvm::MCSubtargetInfo &) const: Assertion `SExpr && SExpr->getSubExpr()->getKind() == MCExpr::SymbolRef && "Unexpected expression in TLS_CALL"' failed.
]

Needs to be investigated.

brad added a subscriber: brad.Jun 23 2018, 6:14 PM

What happened with this? As far as I know this still needs to be fixed.

What happened with this? As far as I know this still needs to be fixed.

As per my latest comment there was an assert when I ran pre-integration testing.
I dont have any immediate plans to continue investigation on that, so if you can take it over from me - please, do.
Functionally this patch was heavily tested as part of our local clang-on-sparc-Solaris installation back at the time when I was part of Oracle Developer Studio project.

The assert does not trigger after https://reviews.llvm.org/rL334383. Is it ok to apply this patch? Just need to change the objdump output from Unknown to the symbol name.

The assert does not trigger after https://reviews.llvm.org/rL334383. Is it ok to apply this patch? Just need to change the objdump output from Unknown to the symbol name.

Ok for me. Are you willing to take the ownership of this?
It will take some time for me to find a functional Sparc machine nowadays.

dcederman commandeered this revision.Sep 3 2018, 1:47 AM
dcederman added a reviewer: fedor.sergeev.

Ok for me. Are you willing to take the ownership of this?

Yes, I can do that. Thanks!

dcederman updated this revision to Diff 163672.Sep 3 2018, 2:21 AM

Rebased and changed objdump output check from Unknown to symbol name (Local/Extern).

This revision was automatically updated to reflect the committed changes.