Page MenuHomePhabricator

[MC] Widen the functional unit type from 32 to 64 bits.
ClosedPublic

Authored by ebevhan on Dec 9 2019, 7:28 AM.

Details

Summary

The type used to represent functional units in MC is
'unsigned', which is 32 bits wide. This is currently
not a problem in any upstream target as no one seems
to have hit the limit on this yet, but in our
downstream one, we need to define more than 32
functional units.

Increasing the size does not seem to cause a huge
size increase in the binary (an llc debug build went
from 1366497672 to 1366523984, a difference of 26k),
so perhaps it would be acceptable to have this patch
applied upstream as well.

Diff Detail

Event Timeline

ebevhan created this revision.Dec 9 2019, 7:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2019, 7:28 AM
jmolloy accepted this revision.Dec 10 2019, 3:01 AM

In general anything moving from 32-bit to 64-bit bitmasks in the backend LGTM. I'd even go further and suggest using std::bitset to remove this problem Once And For All, but that would require significantly more benchmarking.

This revision is now accepted and ready to land.Dec 10 2019, 3:01 AM

In fact, one concrete improvement we could do with this CL (which clearly found everything we needed to patch) is introduce a typedef for "A bitmask of functional units". This would allow us to find all the instances again in future, and highlight the point of the bitwidth to users.

In fact, one concrete improvement we could do with this CL (which clearly found everything we needed to patch) is introduce a typedef for "A bitmask of functional units". This would allow us to find all the instances again in future, and highlight the point of the bitwidth to users.

I considered that. I could make that change.

ebevhan updated this revision to Diff 233107.Dec 10 2019, 7:59 AM

Add a typedef to InstrStage, FuncUnits, and use it as the bitmask type for functional units.

ebevhan updated this revision to Diff 245395.Feb 19 2020, 6:20 AM

Rebase and fix a few places where the FuncUnits type was missing.

This revision was automatically updated to reflect the committed changes.