[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

[Gulfem Savrun Yeniceri via Phabricator via llvm-commits]

gulfem marked 2 inline comments as done.
gulfem added a comment.

In D94355#2492365 <https://reviews.llvm.org/D94355#2492365>, @hans wrote:

> Great stuff, thanks for working on this!

Thanks for the feedback!

In D94355#2492394 <https://reviews.llvm.org/D94355#2492394>, @lebedev.ri wrote:

> Then, tangential question: shouldn't this simply be a separate transformation, to turn non-PIC-friendly (non-relative) LUT's into relative LUT's?

Thanks for the feedback. Do you suggest to implement this as an another optimization after SwitchToLookup in SimplifyCFG?
I consider this optimization is an improvement to an existing SwitchToLookup optimization, and this is why I implemented it as part of that.
What would be the advantage of implementing it as a separate transformation?



================
Comment at: llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h:27
   bool ConvertSwitchToLookupTable = false;
+  bool RelativeSwitchLookupTable = false;
   bool NeedCanonicalLoop = true;
----------------
hans wrote:
> Is it necessary to have an option for this? Can't SwitchToLookupTable just decide itself whether to do this, based on whether it's generating PIC code or not?
> 
> (In any case, I think this only makes sense for PIC code, right?)
Yes, this is only intended for PIC code.
The option is not necessary, but I added it for testing purposes.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:5517
+  // Determines if a relative lookup table generated.
+  bool IsRelative = false;
 };
----------------
hans wrote:
> As far as I understand the flag only applies for ArrayKind, and for that it changes the behaviour quite a bit. Should this be another Kind enumerator instead?
I like that idea, and I think it will make the code more readable if I follow the existing pattern (Using kinds).
I will upload a new revision that uses a new kind for relative lookup tables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355