Page MenuHomePhabricator

GlobalISel: Add computeKnownBitsForTargetInstr

Authored by arsenm on Jan 22 2020, 7:41 AM.



I think we can save the MRI argument from these since it's in
GISelKnownBits already, but currently not accessible.

Implementation deferred to avoid dependency on other patches.

Diff Detail

Event Timeline

arsenm created this revision.Jan 22 2020, 7:41 AM
dsanders added inline comments.Jan 29 2020, 10:46 AM

This TODO hasn't quite been handled. It's also intended to cover the case where you have target specific instructions (e.g. AArch64::ADDWrr), pseudo-instructions, or target-specific generic opcodes.

To cover this case we also need to call TL.computeNumSignBitsForTargetInstr() for the default: case


FirstAnswer is 1 so this if-statement is already inside the std::max


Why update a variable that and return that rather than just returning the answer as per the other code paths?

arsenm marked 2 inline comments as done.Mar 22 2020, 12:00 PM
arsenm added inline comments.

This is called in the default case


This is following the structure of the DAG implementation for eventually plugging in the default handling with computeKnownBits

arsenm updated this revision to Diff 251923.Mar 22 2020, 3:15 PM

Handle the fallback to computeKnownBits, copied directly from the DAG version. This should really get a test, but I don't think there's any operation that is implemented that should hit this path

foad added a subscriber: foad.Mar 23 2020, 2:09 AM
foad added inline comments.

I've simplified the SelectionDAG version of this code a bit in 7cdbf1ed4b94259a3aad2a7575e928fa61b0e57e.

dsanders accepted this revision.Mar 23 2020, 9:45 AM

LGTM after syncing up with @foad's change to use countLeadingOnes() in DAGISel


Ok, that makes sense

This revision is now accepted and ready to land.Mar 23 2020, 9:45 AM