This is an archive of the discontinued LLVM Phabricator instance.

[Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.
ClosedPublic

Authored by craig.topper on Oct 7 2019, 10:50 PM.

Details

Summary

The validateOutputSize and validateInputSize need to check whether
AVX or AVX512 are enabled. But this can be affected by the
target attribute so we need to factor that in.

This patch copies some of the code from CodeGen to create an
appropriate feature map that we can pass to the function. Probably
need some refactoring here to share more code with Codegen. Is
there a good place to do that? Also need to support the cpu_specific
attribute as well.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 7 2019, 10:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 10:50 PM
erichkeane added inline comments.Oct 8 2019, 9:24 AM
clang/include/clang/Basic/TargetInfo.h
948–949

Other Parameter names are commented out (presumably to avoid an unused param warning?). Do we need to do that for this parameter as well?

clang/lib/Sema/SemaStmtAsm.cpp
241

First, this ought to be static. Second, since this is basically identical to what we do for CodeGen, I wonder if this needs to just be a member of Sema/ASTContext or something.

Refactor to share more code with the CodeGen target feature stuff.

rnk accepted this revision.Dec 5 2019, 6:04 PM

lgtm

Hoisting this to ASTContext and using it where needed seems like the logical thing to do.

This revision is now accepted and ready to land.Dec 5 2019, 6:04 PM
This revision was automatically updated to reflect the committed changes.
rnk added inline comments.Dec 6 2019, 3:44 PM
clang/include/clang/AST/ASTContext.h
2846

I reverted this because it requires TargetAttr to be complete here, which I have been working for a few weeks to avoid. If you pull ParsedTargetAttr out of the TargetAttr class, then you can forward declare it, and reland. Sorry I didn't see that in review, this commit raced with rG60573ae6fe509b618dc6a2c5c55d921bccd77608, which removes the Attr.h include in ASTContext.h.

craig.topper reopened this revision.Dec 6 2019, 5:20 PM
This revision is now accepted and ready to land.Dec 6 2019, 5:20 PM
This revision was automatically updated to reflect the committed changes.
MaskRay added inline comments.
clang/include/clang/AST/ASTContext.h
116

This should be struct. I fixed it it in 4b64e034612017fcc97b64d6031319cf18dbbb88

craig.topper marked an inline comment as done.Dec 23 2019, 1:23 PM
craig.topper added inline comments.
clang/include/clang/AST/ASTContext.h
116

Thanks!