This is an archive of the discontinued LLVM Phabricator instance.

ValueTypes.td: Reorganize ValueType to generate `MachineValueType.h`
ClosedPublic

Authored by chapuni on Mar 15 2023, 4:49 PM.

Details

Summary

Introduce VTAny as isOverloaded = true. ValueType.isOverloaded` is used for;

  • Define iPTRAny, vAny, fAny, and Any
  • Reflect ValueType.isOverloaded to LLVMType.isAny in Intrinsics.td
  • Reflect the condition to MVT::isOverloaded()

Introduce some fields in ValueType

  • LLVMName
  • isInteger
  • isFP
  • isVector
  • isScalable
  • nElem
  • ElementType

Introduce VTVec<int nelem, ValueType elt, int value> to represent
the element type. VTVec.Size may be calculated by !mul(nelem, elt.Size).

Introduce list<ValueType> ValueTypes as convention for lookup.

Depends on D146906

Diff Detail

Event Timeline

chapuni created this revision.Mar 15 2023, 4:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 4:49 PM
Herald added a subscriber: ctetreau. · View Herald Transcript
chapuni requested review of this revision.Mar 15 2023, 4:49 PM

One concern. Would it be better that bool fields would be bit rather than int?

One concern. Would it be better that bool fields would be bit rather than int?

Yes but it doesn’t really matter

chapuni updated this revision to Diff 508395.Mar 26 2023, 3:47 AM
chapuni retitled this revision from ValueTypes.td: Reorganize ValueType to ValueTypes.td: Reorganize ValueType to generate `MachineValueType.h`.
chapuni edited the summary of this revision. (Show Details)
  • Fully generate MachineValueType.h

I have deduplicated MachineValueType.h with ValueTypes.td.

It could be relocated to llvm/CodeGen (or llvm/IR), if we could split out CodeGenEmitter stuff from llvm-tblgen. (See also D146352)

kparzysz added inline comments.
llvm/unittests/Support/MachineValueType.h
84 ↗(On Diff #508395)

All of the x ? true : false are equivalent to just x.

arsenm added inline comments.Mar 27 2023, 9:23 AM
llvm/include/llvm/IR/Intrinsics.td
192 ↗(On Diff #508395)

I didn't know tablegen had assertions

llvm/unittests/Support/MachineValueType.h
80 ↗(On Diff #508395)

Have you measured compile time changes? Seems unfortunate to move from range checks to generated switches

craig.topper added inline comments.Mar 27 2023, 9:39 AM
llvm/utils/TableGen/VTEmitter.cpp
78 ↗(On Diff #508395)

Capitalize variable names

chapuni added inline comments.Mar 27 2023, 4:54 PM
llvm/include/llvm/IR/Intrinsics.td
192 ↗(On Diff #508395)

I didn't know it was not older functionality. (Introduced in D93911)

llvm/unittests/Support/MachineValueType.h
80 ↗(On Diff #508395)

I over-expected compiler's optimizations easily.

Re. range checks, I have rethought I could rewind them. Emission of ranges (FIRST/LAST_VALUES) was my latest implementation in VTEmitter. After it, ranges have become credible values provided by ValueTypes.td, since UpdateVTRange (D146906) checks also discontinuous gap.

Re. other generated switches, I will measure impact to compile time and I consider emitting range set expressions instead of generated switches.

84 ↗(On Diff #508395)

I will fix them. I was afraid of int=>bool conversion for no reason. (Guessing comes from restriction of other language)

llvm/utils/TableGen/VTEmitter.cpp
78 ↗(On Diff #508395)

Will fix. Excuse my bad habit.

chapuni updated this revision to Diff 508851.Mar 27 2023, 5:15 PM
chapuni edited the summary of this revision. (Show Details)
  • VTEmitter: Replace the prefix is to Is in vars
  • MachineValueType.h: Rewind generated switches to range checks
  • MachineValueType.h: Get rid of x ? true : false
bjope added a subscriber: bjope.Mar 28 2023, 2:45 PM
chapuni updated this revision to Diff 509853.Mar 30 2023, 4:58 PM
  • Update iwyu
chapuni updated this revision to Diff 511414.Apr 6 2023, 7:37 AM
  • Rebase
chapuni updated this revision to Diff 512134.Apr 10 2023, 6:35 AM
  • Reformat

@arsenm @craig.topper Any other concerns? Could we wait for 3rd opinion?

llvm/unittests/Support/MachineValueType.h
80 ↗(On Diff #508395)

I am certain I wouldn't introduce significant degressions since I rewound expansions of ranges (except for isOverloaded()

arsenm accepted this revision.Apr 24 2023, 1:03 PM
This revision is now accepted and ready to land.Apr 24 2023, 1:03 PM