[PATCH] D142725: [DFAJumpThreading] Enable DFAJT with LTO

Gabor Marton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 27 11:08:49 PST 2023


martong added a comment.

In D142725#4086632 <https://reviews.llvm.org/D142725#4086632>, @nikic wrote:

> In D142725#4086159 <https://reviews.llvm.org/D142725#4086159>, @martong wrote:
>
>> In D142725#4085940 <https://reviews.llvm.org/D142725#4085940>, @nikic wrote:
>>
>>> @martong Would the optimization also work if `-Xclang -enable-dfa-jump-thread` were specified in `CFLAGS` instead? If "yes", then I'd say this is firmly in the realm of user error and we should not make this change.
>>
>> Yes, that would work with `-mllvm -enable-dfa-jump-thread`. And the Coremark score is roughly the same (within measurement error). I agree that it would be better if the user added the DFAJT optimization to CFLAGS rather than to the LDFLAGS, but do we really expect the users to have such a subtle knowledge about the opt passes? In particular, it might be surprising that this opt does not have any effect when it is used with LTO at the moment.
>
> This is an internal LLVM option, it is not intended for end-user consumption. If somebody does want to use it, they had better know what they are doing.
>
> The DFA jump threading pass is intended to be enabled by default in the future, at which point no option will be necessary, and this will just leave behind an (to our knowledge) unnecessary pass run in the LTO pipeline.

Okay, that makes sense to me. There is, however, something that still bothers me. There is the old `JumpThreadingPass` which is added both to the function simplification pipeline and to the LTO pipeline. So,  adding `DFAJumpThreading` to the LTO pipeline would make it consistent with the way how the old `JumpThreadingPass` is added to these pipelines, isn't it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142725/new/

https://reviews.llvm.org/D142725



More information about the llvm-commits mailing list