ego added inline comments.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:827
+ {{"zvkg"}, {ImpliedExtsZve32x}},
+ {{"zvknha"}, {ImpliedExtsZve32x}},
+ {{"zvknhb"}, {ImpliedExtsZve64x}},
----------------
ego wrote:
> How does this work? This doesn't seem to be enough,
> "ImpliedExtsZve32x" does not expand (recursively) to contain "zve32x", which
> is necessary to satisfy "HasVector" in checkDependency(), which leads to an
> error (with some dbgs() statements added in updateImplication()):
>
> > % .. && bin/llvm-lit -v ../llvm/test/CodeGen/RISCV/attributes.ll
> > ...
> > DBG: --- Entering updateImplication
> > DBG: Adding new implied ext >zvknha< => >zvl32b<
> > DBG: --- Exiting updateImplication
> > LLVM ERROR: zvl*b requires v or zve* extension to also be specified
>
> That's because 'ImpliedExtsZve32x' does not include Zve32x, but only the
> extensions implied by Zve32x. So we don't end up with Zve32x being defined.
>
> So I *think* that we either want to imply "zve32x", maybe in a
> ImpliedExtsZvkEW32 (/ ImpliedExtsZvkEW64) to be used by Zvk sub-extensions
> that imply that a 32b-wide SEW is supported (/64b-wide), or we need to ask
> users to specify a vector extension when also declaring a Zvk* extension.
>
I had a chat with Ken Dockser on the topic. Ken's expectation is that one would
have to explicitly declare a vector extension (e.g., 'v', or one of the
"zve<something>") for the Zvk* extensions to become usable. This matches my
previous understanding, probably derived from earlier conversations with Ken.
If this is the intent of the code, the current logic in this file appears to be
appropriate. However the attributes.ll test would then be in error, as it
currently only specifies ```llc -mtriple=riscv32 -mattr=+experimental-zvknha
...```.
As always I might be misinterpreting the intent of this patch. Don't hesitate
to correct me :-) .
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138807/new/
https://reviews.llvm.org/D138807
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
