Page MenuHomePhabricator

[ValueTracking] peek through min/max to find isKnownToBeAPowerOfTwo
ClosedPublic

Authored by spatel on Mar 24 2021, 9:21 AM.

Details

Summary

This is similar to the select logic just ahead of the new code. Min/max choose exactly one value from the inputs, so if both of those are a power-of-2, then the result must be a power-of-2. This might help with D98152, but we likely still need other pieces of the puzzle to avoid regressions.

The part I'm not sure about is the diff in PatternMatch.h. Without that, I get a pile of errors like:
error: assigning to 'llvm::Value *' from incompatible type 'const llvm::Value *'

...and I haven't been able to find a combination of const to avoid it. I did try const_cast<Value *>(V) in the caller code, and that also works. Is that better or worse?

Diff Detail

Event Timeline

spatel created this revision.Mar 24 2021, 9:21 AM
spatel requested review of this revision.Mar 24 2021, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2021, 9:21 AM
lebedev.ri accepted this revision.Mar 24 2021, 9:30 AM

LGTM

llvm/include/llvm/IR/PatternMatch.h
1738–1741

Yeah, i think that's fine..

This revision is now accepted and ready to land.Mar 24 2021, 9:30 AM
RKSimon accepted this revision.Mar 24 2021, 10:01 AM

LGTM with one minor

llvm/include/llvm/IR/PatternMatch.h
1738–1741

Really we should be avoiding auto for anything but casts or iterators

spatel added inline comments.Mar 24 2021, 11:03 AM
llvm/include/llvm/IR/PatternMatch.h
1738–1741

So const_cast in the caller, or do you see another way to avoid the build errors?

RKSimon added inline comments.Mar 24 2021, 1:21 PM
llvm/include/llvm/IR/PatternMatch.h
1738–1741

if its calling build errors then by all means use auto

This revision was landed with ongoing or failed builds.Mar 24 2021, 3:02 PM
This revision was automatically updated to reflect the committed changes.