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

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 13 01:27:52 PST 2021


lebedev.ri added a comment.

> 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'm not really sure where it would fit best. Assuming it doesn't require changing CFG
this might even be eligible for InstCombine, but adding it into simplifycfg is a safe bet i guess.

> 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?



1. It will also catch LUTs that aren't just a about to be created by simplifycfg, but also all those that are already present. I think this is very much in spirit of modular optimizations.
2. All this

  if (IsRelative)
    GenRelative
  else 
    GenAbsolute

looks pretty ad-hoc, as-if some abstraction is missing.
If the transformation doesn't have to know how to generate two different LUT types,
but we have two things doing their own (single) thing, i would expect everything would be less error-prone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355



More information about the llvm-commits mailing list