leonardchan accepted this revision. leonardchan added a comment. This revision is now accepted and ready to land.
LGTM pending a few more comments. Should also give some time to let others respond if they have feedback. ================ Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:41-42 + + if (!dyn_cast<LoadInst>(GEP->use_begin()->getUser())) + return false; + ---------------- Not needed here since you have the `isa` below. ================ Comment at: llvm/test/Transforms/RelLookupTableConverter/relative_lookup_table.ll:2-3 +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt < %s -rel-lookup-table-converter -relocation-model=pic -S | FileCheck %s +; RUN: opt < %s -passes=rel-lookup-table-converter -relocation-model=pic -S | FileCheck %s +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" ---------------- We should also check some other `RUN`s to check that this isn't run on cases that return false in `shouldBuildRelLookupTables`: non-PIC, non-64-bit, other code model sizes, etc. ================ 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( ---------------- 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. ================ Comment at: llvm/utils/gn/secondary/llvm/lib/Transforms/Utils/BUILD.gn:64 "PromoteMemoryToRegister.cpp", + "RelLookupTableConverter.cpp" "SSAUpdater.cpp", ---------------- Good that you added this, but I think Nico has a bot that automatically updates these BUILD.gn files so manually updating them may not be necessary. 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