paulwalker-arm added inline comments.
================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+ assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+ if (LangOpts.VScaleMax)
return std::pair<unsigned, unsigned>(LangOpts.VScaleMin,
LangOpts.VScaleMax);
+
if (hasFeature("sve"))
----------------
c-rhodes wrote:
> paulwalker-arm wrote:
> > This looks like a change of behaviour to me. Previously the command line
> > flags would override the "sve" default but now that only happens when the
> > user specifies a maximum value. That means the interface can no longer be
> > used to force truly width agnostic values.
> > This looks like a change of behaviour to me. Previously the command line
> > flags would override the "sve" default but now that only happens when the
> > user specifies a maximum value. That means the interface can no longer be
> > used to force truly width agnostic values.
>
> I think the issue here is the default of 1 for min would always trigger `if
> (LangOpts.VScaleMin || LangOpts.VScaleMax)` overriding the SVE default.
> Perhaps the default can be removed from the driver option and handled here,
> i.e.
>
> ```
> if (LangOpts.VScaleMin || LangOpts.VScaleMax)
> return std::pair<unsigned, unsigned>(LangOpts.VScaleMin ?
> LangOpts.VScaleMin : 1,
> LangOpts.VScaleMax);
> ```
>
>
Is this enough? I'm not sure it'll work because `LangOpts.VScaleMin` defaults
to 1 and thus you'll always end up passing the first check, unless the user
specifically uses `-mvscale-min=0` which they cannot because that'll result in
`diag::err_cc1_unbounded_vscale_min`.
Do we need to link the LangOpt defaults to the attribute defaults? I'm think
that having the LangOpts default to zero is a good way to represent "value is
unspecified" with regards to the `LangOpts.VScaleMin`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113294/new/
https://reviews.llvm.org/D113294
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
