Page MenuHomePhabricator
Differential D145873

Move definitions of ArgKind from Intrinsics.h to Intrinsics.td
ClosedPublic
Actions

Authored by chapuni on Sun, Mar 12, 7:47 AM.

Details

Reviewers
rnk
craig.topper
arsenm
Commits
rG8922a1c15ba2: Move definitions of ArgKind from Intrinsics.h to Intrinsics.td
Summary

Let IntrinsicEnums.inc conditional by GET_INTRINSIC_ENUM_VALUES

Move definitions of ArgKind from Intrinsics.h to Intrinsics.td

Values of ArgKind are used (as naked constants) also in IntrinsicEmitter.
They can be dissolved to move their logic to Intrinsics.td.

Diff Detail

Event Timeline

chapuni created this revision.Sun, Mar 12, 7:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Mar 12, 7:47 AM
chapuni requested review of this revision.Sun, Mar 12, 7:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Mar 12, 7:47 AM
Herald added subscribers: llvm-commits, jdoerfert. · View Herald Transcript
chapuni retitled this revision from Let IntrinsicEnums.inc conditional by GET_INTRINSIC_ENUM_VALUES to Move definitions of ArgKind from Intrinsics.h to Intrinsics.td.Sun, Mar 12, 7:57 AM
chapuni edited the summary of this revision. (Show Details)
chapuni added a reviewer: rnk.
Harbormaster completed remote builds in B218877: Diff 504437.Sun, Mar 12, 8:31 AM
aganea added a subscriber: aganea.Sun, Mar 12, 5:13 PM

Reid is out for vacations, I’d suggest perhaps to find another reviewer!

chapuni mentioned this in D145937: Replace `IntrinsicEmitter::ComputeFixedEncoding()` and cleanup.Mon, Mar 13, 7:13 AM
chapuni added a reviewer: craig.topper.Tue, Mar 14, 3:33 PM
chapuni added a subscriber: craig.topper.

@craig.topper Could you take a look?

Is this the right direction to unify constant definitions?
See also, https://discourse.llvm.org/t/rfc-let-intrinsics-td-emit-iit-info-typesig/69186
I am planning also to move also IIT_Info constants from Function.cpp to Intrinsics.td.

craig.topper added inline comments.Tue, Mar 14, 4:05 PM
llvm/utils/TableGen/IntrinsicEmitter.cpp
175

Rec confused me at first. These aren't Records. I think they are RecordVal? RV or Field would be a better name.

chapuni updated this revision to Diff 505326.Tue, Mar 14, 4:59 PM
  • EmitArgKind: s/Rec/RV/g
chapuni added inline comments.Tue, Mar 14, 5:01 PM
llvm/utils/TableGen/IntrinsicEmitter.cpp
175

Makes sense. I was confused, too.

Harbormaster completed remote builds in B219520: Diff 505326.Tue, Mar 14, 5:45 PM
nikic added a subscriber: nikic.Wed, Mar 15, 1:44 AM

I'm having trouble understanding what the point of this change is. Is there some change on top of this that will make use of ArgKind from tablegen?

chapuni added a comment.Wed, Mar 15, 5:51 AM

@nikic Pre-stage changes are D145937 and https://github.com/chapuni/llvm-project/commits/tblgen/typesig

chapuni added a child revision: D146914: Migrate `IIT_Info` into `Intrinsics.td`.Sun, Mar 26, 6:58 AM
arsenm accepted this revision.Thu, Mar 30, 11:28 AM
arsenm added a subscriber: arsenm.

LGTM. I"m assuming the other diffs somehow make this worthwhile since currently there's only 5 values

This revision is now accepted and ready to land.Thu, Mar 30, 11:28 AM
Herald added a subscriber: wdng. · View Herald TranscriptThu, Mar 30, 11:28 AM
chapuni added a comment.Thu, Mar 30, 3:15 PM

As you knew, this is for D146915.

This revision was landed with ongoing or failed builds.Thu, Mar 30, 5:15 PM
Closed by commit rG8922a1c15ba2: Move definitions of ArgKind from Intrinsics.h to Intrinsics.td (authored by chapuni). · Explain Why
This revision was automatically updated to reflect the committed changes.
chapuni mentioned this in rGa365fc4e56f1: Let IntrinsicEnums.inc conditional by GET_INTRINSIC_ENUM_VALUES.
chapuni added a commit: rG8922a1c15ba2: Move definitions of ArgKind from Intrinsics.h to Intrinsics.td.

Revision Contents
Changeset List

PathSize
llvm/
include/
llvm/
IR/
10 lines
15 lines
utils/
TableGen/
18 lines

Diff 509858

View Options

llvm/include/llvm/IR/Intrinsics.h

Loading...
View Options

llvm/include/llvm/IR/Intrinsics.td

Loading...
View Options

llvm/utils/TableGen/IntrinsicEmitter.cpp

Loading...