gulfem marked 4 inline comments as done.
gulfem added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:32
+ // If lookup table has more than one user,
+ // do not generate a relative lookup table.
+ if (!GV.hasOneUse())
----------------
hans wrote:
> It would be better if the comment said why. I suppose the reason is we need
> to be sure there aren't other uses of the table, because then it can't be
> replaced.
>
> But it would be cool if a future version of this patch could handle when
> there are multiple loads from the table which can all be replaced -- for
> example this could happen if a function which uses a lookup table gets
> inlined into multiple places.
I actually ran into the exact case that you described during testing, where a
function that uses a switch gets inlined into multiple call sites :)
This is only to simplify the analysis, and I now added a TODO section to
explore that later.
================
Comment at:
llvm/test/Transforms/RelLookupTableConverter/switch_relative_lookup_table.ll:39
+; ; Relative switch lookup table for strings
+define i8* @string_table(i32 %cond) {
+ ; FNOPIC-LABEL: @string_table(
----------------
leonardchan wrote:
> It looks like this test case isn't much different from `string_table` in
> `relative_lookup_table.ll`? If so, then this file could be removed.
I renamed this test case to no_relative_lookup_table.ll that checks the cases
where relative lookup table should not be generated like in non-pic mode,
medium or large code models, and 32 bit architectures, etc.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94355/new/
https://reviews.llvm.org/D94355
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
