[PATCH] D70095: [PGO][PGSO] DAG.shouldOptForSize part.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 21 11:45:37 PST 2019


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

In D70095#1755505 <https://reviews.llvm.org/D70095#1755505>, @yamauchi wrote:

> In D70095#1754675 <https://reviews.llvm.org/D70095#1754675>, @RKSimon wrote:
>
> > Does WebAssembly still need updating ?
>
>
> I somehow missed the WebAssembly comment. Thanks for reminding.
>
> I think we can remove WebAssemblyDAGToDAGISel::ForCodeSize and the hasOptSize call.
>  Three reasons:
>  (1) ForCodeSize is not used,
>  (2) We can't call DAG.shouldOptForSize there because DAG isn't initialized at that point (if we want to query this at a block granularity, it wouldn't make sense to call it there to begin with.)
>  (3) Removing it would minimize confusions, I think, in case we want to implement size optimizations here for WebAssembly in the future (in which case, we could follow the other targets code and use DAG.shouldOptForSize.)


LGTM.
I agree that removing unused code is good, but I've never looked at WebAssembly before, so let's see if anyone else has comments (wait a day to commit).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70095





More information about the llvm-commits mailing list