Page MenuHomePhabricator

[VectorUtils] API for VFShape, update VFInfo.

Authored by fpetrogalli on Nov 20 2019, 2:34 PM.


This patch introduces an API to build and modify vector shapes.

The validity of a VFShape can be checked with the
`hasValidParameterList` method, which is also run in an assertion each
time a VFShape is modified.

The field VFISAKind has been moved to VFInfo under the assumption that
different ISAs can map to the same VFShape (as it can be in the case
of vector extensions with the same registers size, for example AVX and

Reviewers: sdesmalen, jdoerfert, simoll, hsaito

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision:

Diff Detail

Event Timeline

fpetrogalli created this revision.Nov 20 2019, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2019, 2:34 PM
fpetrogalli marked an inline comment as done.Nov 20 2019, 2:39 PM
fpetrogalli added inline comments.

This should be /*Parameters*/, not /*HasGlobalPredicate*/. Same for all uses below.

fpetrogalli marked an inline comment as not done.Nov 20 2019, 2:39 PM
fpetrogalli marked an inline comment as done.

Fully implement and test the API.

This patch is ready to be reviewed.



In this patch I have achieved better unit tests granularity by moving
the EXPECT_* macros from the test fixture class into the actual
test, so that in case of failure the log message points at the right
line in the code (otherwise it always points at the EXPECT_* in the
method used to run the check in the class definition of the test

fpetrogalli retitled this revision from [VectorUtils] API for VFShape. to [VectorUtils] API for VFShape, update VFInfo..Nov 20 2019, 8:08 PM
fpetrogalli edited the summary of this revision. (Show Details)
fpetrogalli marked an inline comment as not done.
fpetrogalli marked an inline comment as done.

In this last update I have:

  1. Removed an assertion in updateParameter that was checking a condition for VFParamKind::GlobalPredicate already checked in hasValidParameterList.
  1. Renamed a method in the test for better readability of the code (from isValidParameterList to validParams).



In this update I have removed empty statements in the tests. (facepalm)



Update (and rename) the get method to use ElementCount instead of VF
and IsScalable separately.

fpetrogalli marked an inline comment as done.Nov 22 2019, 9:50 AM
fpetrogalli added inline comments.

Notice that I am not using ElementCount here to realize VF and IsScalable because it would delete the default constructors associated to VFShape and VFInfo.

Looks fine from my perspective. Anyone else has input on this?

Allow GlobalPredicate parameters in any position of the signature.

sdesmalen added inline comments.Dec 4 2019, 8:59 AM

nit: can you add a newline between the previous and next method?


should this be an assert?


nit: can you add a newline above?


nit: Nothign -> Nothing


Can you add a function here that calculates this, per supported VFABI?


nit: this could do with some newlines.

fpetrogalli marked 8 inline comments as done.

Address review comments from @sdesmalen.


I have updated also the tests replacing EXPECT_FALSE with EXPECT_DEATH, to make sure the assert is not removed.


Marking this as done as the most recent version of the patch doesn't have any concept for VFABI for GlobalPredicate, as the global predicate is allowed to be anywhere in the signature, as long as it is unique.

sdesmalen accepted this revision.Dec 4 2019, 11:53 AM

Thanks for the fixes @fpetrogalli, LGTM!

This revision is now accepted and ready to land.Dec 4 2019, 11:53 AM
This revision was automatically updated to reflect the committed changes.