gulfem added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:73
+ return false;
+
+ // If an operand in the lookup table is not dso_local,
----------------
pcc wrote:
> In the version of the patch that you committed, you have this check here:
> ```
> // If operand is mutable, do not generate a relative lookup table.
> auto *GlovalVarOp = dyn_cast<GlobalVariable>(GVOp);
> if (!GlovalVarOp || !GlovalVarOp->isConstant())
> return false;
> ```
> 1. Nit: Gloval -> Global
> 2. Why is it important whether the referenced global is mutable? The pointer
> itself is constant.
1. That's a typo, and I will fix that.
2. This optimization does not do a detailed analysis, and it is being
conservative.
In this case, `GlobalVar` points to the switch lookup table and
`GlobalVarOp` points to the elements in the lookup table like strings.
To make sure that relative arithmetic works, it just checks whether both
`GlobalVar` and `GlobalVarOp` pointers are constants.
Did you see an issue on that?
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
