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

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+    if (!TM.getTargetTriple().isArch64Bit())
+      return false;
+
----------------
lebedev.ri wrote:
> 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?
`isArch64Bit` checks whether we have a 64-bit architecture, right?
I don't think it specifically checks for `AArch64`, and it can cover other 64-bit architectures like `x86_64` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355