[PATCH] D126089: [WPD] Try harder to find assumes through phis
Arthur Eubanks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 9 13:57:31 PDT 2022
aeubanks added a comment.
> Since `-lto-whole-program-visibility` is a flag that affects the operation of the type tests globally, I think a better design would be to replace all `llvm.type.test` calls with `true`, and replaces `llvm.type.checked.load` with a regular load, if the flag is not passed.
My original patch (https://reviews.llvm.org/D126089?id=431037) replaced `llvm.type.test` with `true` in all cases, but tejohnson said
> Regarding the quoted comment, note it is "type test assume" sequences, not all type tests, that need to be removed here in certain circumstances. Note also that before I made the change to try to keep these around longer for use by other analyses, this code attempted to remove all "type test assume" sequences. I.e. it removed all the analyzed assumes, and any type test that was subsequently dead. Presumably that version of WPD+LTT would have also illustrated the original problem this patch tries to address (an assume fed by a phi, in turn fed by a type test that got an Unsat and resulted in llvm.assume(false)), and for a WPD+CFI build we certainly wouldn't want to replace *all* type test uses in the code at this point with true. So I am skeptical that it is correct to replace type test non-assume (non-phi) uses with true in the more limited cases being handled currently either.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126089/new/
https://reviews.llvm.org/D126089
More information about the llvm-commits
mailing list