This is an archive of the discontinued LLVM Phabricator instance.

fix TLI's combineRepeatedFPDivisors interface to return the minimum user threshold
ClosedPublic

Authored by spatel on Jul 27 2015, 11:30 AM.

Details

Summary

This fix was suggested as part of D11345.

We can avoid walking the uses of a divisor node if the target doesn't want the combineRepeatedFPDivisors transform in the first place.

This is a NFC-intended patch, but I want to make sure the new hook is named and defined as expected. I thought about returning a single 'unsigned' value and treating '0' as a special case for "don't do this transform", but that seems a bit dirty.

Diff Detail

Event Timeline

spatel updated this revision to Diff 30710.Jul 27 2015, 11:30 AM
spatel retitled this revision from to fix TLI's combineRepeatedFPDivisors interface to return the minimum user threshold.
spatel updated this object.
spatel added reviewers: hfinkel, chandlerc.
spatel added a subscriber: llvm-commits.

Looks reasonable to me, although I don't often touch that code.

Looks reasonable to me, although I don't often touch that code.

Thanks, Alexey! Can I use that as an official LGTM? :)

Either way, I'll wait a bit to see if Chandler or Hal has feedback since they commented earlier.

chandlerc accepted this revision.Jul 28 2015, 3:20 PM
chandlerc edited edge metadata.

LGTM with one tweak.

If others want to keep bike-shedding this TTI interface, fine, but let's get this in because PR24141 is hitting us.

include/llvm/Target/TargetLowering.h
2738

Please make this return the minimum number of uses that should exist for this combine. "zero" is a pretty clear way to disable this IMO, as there is never a reason to combine anything with zero uses.

This revision is now accepted and ready to land.Jul 28 2015, 3:20 PM
This revision was automatically updated to reflect the committed changes.