Page MenuHomePhabricator

AArch64: add support for llvm.aarch64.hint intrinsic
ClosedPublic

Authored by compnerd on Jul 6 2014, 3:09 PM.

Details

Summary

This adds a llvm.aarch64.hint intrinsic to mirror the llvm.arm.hint in order to
support the various hint intrinsic functions in the ACLE.

Add an optional pattern field that permits the subclass to specify the pattern
that matches the selection. The intrinsic pattern is set as mayLoad, mayStore,
so overload the value for the definition of the hint instruction.

Diff Detail

Event Timeline

compnerd updated this revision to Diff 11108.Jul 6 2014, 3:09 PM
compnerd retitled this revision from to AArch64: add support for llvm.aarch64.hint intrinsic.
compnerd updated this object.
compnerd edited the test plan for this revision. (Show Details)
compnerd added a reviewer: t.p.northover.
compnerd set the repository for this revision to rL LLVM.
compnerd added a subscriber: Unknown Object (MLST).
compnerd updated this revision to Diff 11109.Jul 6 2014, 4:45 PM
compnerd updated this object.
compnerd added a reviewer: joey.

Thanks to Joey for having me retry the changing the mayLoad, mayStore properties of the hint instruction definition.

kongyi added a subscriber: kongyi.Jul 7 2014, 12:25 AM

I believe modelling hints intrinsics to mayLoad and mayStore is not accurate as they don't actually load or store. Modelling as

mayLoad = ?
mayStore = ?

should be better.

compnerd updated this revision to Diff 11141.Jul 7 2014, 10:18 PM

Address review comments from Yi.

t.p.northover edited edge metadata.Jul 8 2014, 8:17 AM

I don't think the "mayLoad = ?" is any better. TableGen is still inferring "mayLoad = 1", we're just concealing that from the reader.

I think I'd go for the original code, with a comment that really these should be "mayLoad = 0" but that intrinsics' side effects aren't modelled with enough granularity for that to be possible.

Cheers.

Tim.

Is hasSideEffects=1 not sufficient?

-j

Unfortunately, it doesn't. tablgen infers that mayLoad and mayStore should be 1 for the pattern, but the instruction is not mayLoad and not mayStore.

compnerd updated this revision to Diff 11181.Jul 8 2014, 9:30 PM
compnerd edited edge metadata.
t.p.northover accepted this revision.Jul 11 2014, 8:39 AM
t.p.northover edited edge metadata.

I think this looks fine now. Thanks for putting up with our dithering.

Cheers.

Tim.

This revision is now accepted and ready to land.Jul 11 2014, 8:39 AM
compnerd closed this revision.Jul 12 2014, 6:13 PM

Committed as SVN r212883.