[PATCH] D143619: [llvm][codegen] Disallow default Emulated TLS for RISCV

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added a comment.

In D143619#4117333 <https://reviews.llvm.org/D143619#4117333>, @vit9696 wrote:

> This change looks inadequate to me. As I explicitly stated in the mentioned issue <https://github.com/llvm/llvm-project/issues/59500#issuecomment-1349012654>, we need emutls support for RISC-V. For the reasons stated there I believe we should enable emutls support for RISC-V as GCC does, and this differential should be abandoned.
>
> Just to clarify, emutls support is literally as simple as adding these three lines:
>
>   diff -rupN a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
>   --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp	2022-06-22 19:46:24
>   +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp	2022-12-14 11:48:02
>   @@ -3718,6 +3718,9 @@ SDValue RISCVTargetLowering::lowerGlobalTLSAddress(SDV
>    
>    SDValue RISCVTargetLowering::lowerGlobalTLSAddress(SDValue Op,
>                                                       SelectionDAG &DAG) const {
>   +  GlobalAddressSDNode *GA = cast<GlobalAddressSDNode>(Op);
>   +  if (DAG.getTarget().useEmulatedTLS())
>   +    return LowerToTLSEmulatedModel(GA, DAG);
>      SDLoc DL(Op);
>      EVT Ty = Op.getValueType();
>      GlobalAddressSDNode *N = cast<GlobalAddressSDNode>(Op);

The code owners of the RISCV backend seem to disagree as pointed out in that issue and the linked differential on phabricator.

As it stands we miscompile code that sets these options. Until the RISC-V backend correctly handles TLS emulation we should prevent such issues.  If the change you suggest lands, this can be reverted.

Until then we should not ship compilers that work incorrectly and that silently miscompile user code. I think maintaining that invariant is much more important than parity or conformity with other compilers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143619/new/

https://reviews.llvm.org/D143619