[PATCH] D69618: NeonEmitter: clean up prototype modifiers

Tim Northover via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 31 01:16:41 PDT 2019


t.p.northover added a comment.

> It looks like this patch contains a few other changes, besides the changes to the prototypes. In particular, the change to CGBuiltin.cpp, and there are a few new lines in the .td files that don't correspond to anything in the old versions. Is that accidental, or is it part of cleaning up the prototypes, somehow?

The extra .td lines are because just those 3 intrinsics used a fixed-width modifier ("give me half, no matter the input") with multiple sizes of input so there's no way to represent that in the new scheme and they need to be split up. Notice the integer ones are already split up because there was no corrresponding "give me int32_t" modifier. That change is actually already a separate NFC commit in my local repository and I'd commit it like that so that the script worked cleanly.

The CGBuiltin change follows from dropping the heuristic hasFloatingProtoModifier when deciding what type to pass to CGBuiltin for the intrinsics. This affected vmulx and the vcvt intrinsics. In vcvt's case I eventually decided to support them by moving to an explicit '!' modifier and special-casing conversion because they make good use of having signedness on the type they're given. I didn't revisit vmulx after that change, but I'd be inclined to leave it as it is; I kind of think it's unlikely someone implementing that now would make use of the ! modifier, which seems like a pretty rare requirement.

There are two other things that I think are pretty straightforward, but do clutter this patch so I'll split them out: removing the special behaviour of 'a' (it can be implemented in .td at a net -ve lines); and changing Type to use an enum instead of a series of bools. I'll upload new diffs and update this one.

The other


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69618/new/

https://reviews.llvm.org/D69618





More information about the cfe-commits mailing list