Page MenuHomePhabricator

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

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



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):

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

Unit TestsFailed

340 msx64 debian > XRay-x86_64-linux.TestCases/Posix::basic-filtering.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fxray-instrument -m64 -std=c++11 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/xray/TestCases/Posix/basic-filtering.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/xray/X86_64LinuxConfig/TestCases/Posix/Output/basic-filtering.cpp.tmp -g

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


spatel added inline comments.Dec 1 2021, 8:49 AM

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.


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.