[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