This is an archive of the discontinued LLVM Phabricator instance.

[RFC][DAG] Match select of constant equivalents in foldBinOpIntoSelect.
Needs ReviewPublic

Authored by deadalnix on Jul 27 2022, 4:45 PM.

Details

Summary

This makes the combiner more robust in case where the select is optimized away because the binary op is processed. This effect is visible in D127115 .

The diff as this has a couple of regression, but I want to get feedback on the apporach to know if tthis is worth going throught he trouble of nailing them down.

Diff Detail

Event Timeline

deadalnix created this revision.Jul 27 2022, 4:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2022, 4:45 PM
deadalnix requested review of this revision.Jul 27 2022, 4:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2022, 4:45 PM
RKSimon added inline comments.Jul 28 2022, 2:36 AM
llvm/test/CodeGen/VE/Scalar/atomic.ll
23

regenerate and NFC commit to reduce the diff

foad added a subscriber: foad.Jul 28 2022, 3:43 AM

The AMDGPU diff looks nice!

I've not strong objection, but I'd like to see how far you can get fixing the x86 combineSelectOfTwoConstants combines

deadalnix added inline comments.Jul 28 2022, 5:58 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

@RKSimon Is this an improvement or a regression? Or it doesn't matter?

RKSimon added inline comments.Jul 28 2022, 6:12 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

Avoiding cmov for similar codesize is usually a win - they can't be predicted and have tight eflags dependencies

gchatelet added inline comments.
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

The codegen of llvm-libc memcmp and bcmp should directly benefit from it :-) Thx!

gchatelet added inline comments.Jul 28 2022, 6:27 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

Ha sorry I read it in the opposite direction. So this would be a regression...

I don't have a problem with the direction, but as noted earlier, all of the x86 tests that changed to cmov would likely be viewed as regressions. Maybe we can predicate with the phase of combining to limit those diffs?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2256

Does this need to use TLI.getBooleanContents() for non i1 types?

deadalnix added inline comments.Jul 28 2022, 9:46 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

I'm working on a fix for this :) unfortunately, it creates some regressions in the powerpc target, so it's a bit like playing wack a mole.

deadalnix added inline comments.Jul 28 2022, 9:47 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
2256

Is here a predicate to check if something is a "not"?

deadalnix added inline comments.Jul 29 2022, 5:44 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

So D130765 fixes this, but in turn seems to introduce some regressions for PowerPC.

deadalnix added inline comments.Sep 27 2022, 12:40 AM
llvm/test/CodeGen/X86/memcmp-more-load-pairs-x32.ll
165

The regression for PowerPC are gone, I'm going to try to rebase this.

@deadalnix reverse-ping - rebase this?

chfast added a subscriber: chfast.Nov 5 2022, 11:38 AM