This is an archive of the discontinued LLVM Phabricator instance.

[MIRParser] Parse lane masks in block liveins
ClosedPublic

Authored by kparzysz on Oct 12 2016, 12:47 PM.

Details

Summary

Add a token HexLiteral that contains uninterpreted hexadecimal string that could be either an integer, or a floating-point representation. When parsing block liveins, if a register name is followed by :, expect a lane mask value (either decimal or hexadecimal with 0x).

As it is now (even before this patch), there is no way to tell whether a hex value corresponds to an integer, or a floating-point. If floating-point literals were required to have a special prefix (which would be really, really nice), one could disambiguate between integers and floating-point values by just looking at the literal (which would be even nicer). This would remove the need for HexLiteral, which now needs to be interpreted further based on what is expected (which is not quite as nice as it could be).

May I suggest that floating-point hexadecimal values are required to have such a prefix? Because that would be really nice. :)

Diff Detail

Repository
rL LLVM

Event Timeline

kparzysz updated this revision to Diff 74421.Oct 12 2016, 12:47 PM
kparzysz retitled this revision from to [MIRParser] Parse lane masks in block liveins.
kparzysz updated this object.
kparzysz added a reviewer: MatzeB.
kparzysz set the repository for this revision to rL LLVM.
kparzysz added a subscriber: llvm-commits.
MatzeB accepted this revision.Oct 12 2016, 1:23 PM
MatzeB edited edge metadata.

Have you seen https://reviews.llvm.org/D24208? This here is a good solution for that problem, LGTM.

lib/CodeGen/MIRParser/MILexer.cpp
449–452

If we find one of those hex prefixes we could go to FloatingPointLiteral immediately.

lib/CodeGen/MIRParser/MIParser.cpp
1150–1152

I don't like relying on numeric_limits<unsigned> here and would rather hardcode 32 to get consistent behaviour even if a platform has bigger unsigned numbers. But I realize this is in line with the existing code so we can fix that in a separate commit.

This revision is now accepted and ready to land.Oct 12 2016, 1:23 PM
kparzysz marked an inline comment as done.Oct 12 2016, 2:14 PM
This revision was automatically updated to reflect the committed changes.