[PATCH] D156292: [PowerPC] Support initial-exec TLS relocation on AIX

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

It is not clear how someone in the future should react if the test case changes behaviour. You should think about maintainability of the test cases. It would appear that a part of the checks were produced using a script and a part was added manually. As such, it is not clear what the essential parts of the tests are so anyone maintaining this in the future must really dive deep into the TLS implementation to understand if changes are safe or not. I'm not sure what the best approach would be. Perhaps add a comment describing what is being tested. Or maybe only test the key parts using manual checks.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:840
+        return MCSymbolRefExpr::VariantKind::VK_PPC_AIX_TLSIE;
       llvm_unreachable("Only expecting local-exec accesses!");
     }
----------------
This message should probably be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156292