[PATCH] D62960: Add SVE opaque built-in types

This revision was automatically updated to reflect the committed changes. Closed by commit rL368413: Add SVE opaque built-in types (authored by rsandifo, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D62960: Add SVE opaque built-in types

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/AST/ASTContext.cpp:1974 + break; +#include "clang/Basic/AArch64SVEACLETypes.def" } rsandifo-arm wrote: > rjmccall wrote:

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm marked 7 inline comments as done. rsandifo-arm added inline comments. Comment at: lib/AST/ASTContext.cpp:1974 + break; +#include "clang/Basic/AArch64SVEACLETypes.def" } rjmccall wrote: > Why do SVE predicates have 16-bit alignment? Should

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm updated this revision to Diff 214095. rsandifo-arm edited the summary of this revision. rsandifo-arm added a comment. - Remove pointless "SVE Types" comments - Expand commentary about SVE type layout and future debug info handling - Make macro formatting more consistent with the

[PATCH] D62960: Add SVE opaque built-in types

rjmccall added inline comments. Comment at: lib/AST/ASTContext.cpp:1974 + break; +#include "clang/Basic/AArch64SVEACLETypes.def" } Why do SVE predicates have 16-bit alignment? Should this be 128-bit (16-*byte*)? I guess these alignments are

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm added a comment. Ping. @rjmccall: thanks for the positive feedback. Is there a particular reviewer you'd like to hear from before the patch lands? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm marked 10 inline comments as done. rsandifo-arm added a comment. Thanks. I think the current version of the patch addresses all review comments so far and the last set of comments seemed positive. Does the patch look OK to land? (I was waiting until Clang 9 branched, and then

[PATCH] D62960: Add SVE opaque built-in types

rovka added a comment. FWIW, I think the tests look great. Would be nice if someone more experienced with clang could also have a look though. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960

[PATCH] D62960: Add SVE opaque built-in types

rjmccall added a comment. Seems reasonable to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm added a comment. Ping, and thanks for the reviews so far. I think I've addressed all the comments to date and I've tried to make the patch ready to commit in its current state (rather than the RFC that it originally was). It would be really useful if we could apply the patch

[PATCH] D62960: Add SVE opaque built-in types

rjmccall added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:2680 +break; +#include "clang/Basic/AArch64SVEACLETypes.def" } rsandifo-arm wrote: > rovka wrote: > > erik.pilkington wrote: > > > rsandifo-arm wrote: > > > > erik.pilkington wrote: > >

[PATCH] D62960: Add SVE opaque built-in types

martong added a comment. The ASTImporter and the test for it looks good to me, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 ___ cfe-commits mailing list

[PATCH] D62960: Add SVE opaque built-in types

rsandifo-arm added a comment. In D62960#1566265 , @rovka wrote: > This looks much better, thanks! Shouldn't there be more tests, e.g. for > mangling and maybe the ASTImporter? Thanks. I've added tests for ASTImporter and mangling in the latest