[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+    if (!TM.getTargetTriple().isArch64Bit())
+      return false;
+
----------------
gulfem wrote:
> lebedev.ri wrote:
> > 1. But all tests are using `x86_64` triple?
> > 2. This is somewhat backwards. if the target wants to disable this, it will need to override this function with `return false;`.
> 1. Although I used `x86_64 triple`, this optimization can be applied to other 64-bit architectures too, because it not target dependent except `isArch64Bit` and `getCodeModel` check.
> 2. Is there a target that you have in mind that we need to disable this optimization? 
> I thought that it makes sense to enable this optimization by default on all the targets that can support it.
> In case targets want to disable it, they can override it as you said.
> How can we improve the implementation?
> If you have suggestions, I'm happy to incorporate that.
> 
I'm sorry, i do not understand.
Why does `!TM.getTargetTriple().isArch64Bit()` check exist?
To me it reads as "if we aren't compiling for AArch64, don't build rel lookup tables".
Am i misreading this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355