Page MenuHomePhabricator

[PatternMatch] create and use matcher for 'not' that excludes undef elements
ClosedPublic

Authored by spatel on Dec 1 2021, 8:31 AM.

Details

Summary

We needed a stricter version of m_Not for D114462, but I wasn't sure if that was going to be required anywhere else, so I didn't bother to make that reusable.

It turns out we have one more existing simplification that needs this (currently miscompiles):
https://alive2.llvm.org/ce/z/9-nTKi

And there's at least one more fold in that family that we could add.

I copied from existing code to implement this, but I'm definitely open to suggestions if there's a better way. I don't know C++ template magic well at all.

Diff Detail

Event Timeline

spatel created this revision.Dec 1 2021, 8:31 AM
spatel requested review of this revision.Dec 1 2021, 8:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2021, 8:31 AM
lebedev.ri added inline comments.Dec 1 2021, 8:40 AM
llvm/include/llvm/IR/PatternMatch.h
2295–2298

?

spatel added inline comments.Dec 1 2021, 8:49 AM
llvm/include/llvm/IR/PatternMatch.h
2295–2298

That's how I originally implemented it. :)

But that mismatched on the unit test that I created. In that test, the other operand is a zero constant with no undefs. So the commutative matcher matched the zero constant in that example as C and then failed the subsequent isAllOnes check.

It seems unlikely that we'd see something like that (should get constant-folded before we see it) in the optimizer, but it's still possible in general usage?

This revision is now accepted and ready to land.Dec 1 2021, 8:54 AM
rampitec accepted this revision.Dec 1 2021, 11:08 AM

LGTM. Comment about commuted xor would be handy.

llvm/include/llvm/IR/PatternMatch.h
2295–2298

Sounds like a good thing to leave comment here. I also thought about m_c_Xor.

spatel updated this revision to Diff 391138.Dec 1 2021, 2:19 PM

Added code comment to explain commutable corner case and block comment for the matcher.

This revision was landed with ongoing or failed builds.Dec 2 2021, 5:51 AM
This revision was automatically updated to reflect the committed changes.