sdesmalen added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:939
+def warn_pragma_attribute_scalable_unused : Warning<
+ "ignoring scalable vectorize_width flag due to lack of target support">,
+ InGroup<PragmaClangAttribute>;
----------------
From what I can see, the vectorize_width flag is not ignored, only the scalable
property is. That means this should be:
'scalable' not supported by the target so assuming 'fixed' instead.
================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:148
+ if (StateLoc && StateLoc->Ident && StateLoc->Ident->isStr("scalable")) {
+ if (!S.Context.getTargetInfo().supportsScalableVectors()) {
+ S.Diag(St->getBeginLoc(),
diag::warn_pragma_attribute_scalable_unused);
----------------
If the target does not support scalable vectors, it currently assumes
`"fixed"`. If we want to stick with that approach, the diagnostic message
should be changed (see my other comment). The alternative is dropping the hint
entirely by returning `nullptr` and changing the diagnostic message to say the
hint is ignored. I could live with both options. @fhahn do you have a
preference here?
nit: to reduce nesting, can you hoist this out one level, e.g.
if (StateLoc && StateLoc->Ident & ...)
State = LoopHintAttr::ScalableNumeric;
else
State = LoopHintAttr::Numeric;
if (State == LoopHintAttr::ScalableNumeric &&
!S.Context.getTargetInfo().supportsScalableVectors()) {
S.Diag(....);
State = LoopHintAttr::Numeric;
}
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89031/new/
https://reviews.llvm.org/D89031
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
