[PATCH] D136106: [clang][RISCV] Set vscale_range attribute based on VLEN

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D136106#3863202 <https://reviews.llvm.org/D136106#3863202>, @reames wrote:

> In D136106#3863147 <https://reviews.llvm.org/D136106#3863147>, @craig.topper wrote:
>
>>> In the original review, I'd mentioned that MinVLEN was sometimes zero. That's still true, but apparently only happens if you specify multiple extensions in the -target_feature string. So, we can at least test "v" and "zve64x" on their own before I go figure out exactly what's going wrong regarding e.g. parsing "+v,+zvl512b".
>>
>> I think you need to use `-target-feature "v" -target-feature "zvl512b".
>
> So, this was part right.  The other issue is I'd been using Zvl512b, and apparently this bit of parsing code is case sensitive.  I'm going to look into improving the error reporting here.

Replying back to myself here as I'm not quite sure where to put this...

So, it turns out target-feature is currently quite weird.  At the *clang* level, we require individual target-feature options for each extensions.  Using a compound "+v,+zvl512b" is not recognized by the clang code (i.e, the bit this patch was modifying).  However, this raw string gets joined with the valid attributes, and set into the IR as "target-features"="+64bit,+v,+zvl512b" on the function.  This string is then parsed by later consumers, and thus the extensions listed in the compound list *are* picked up during e.g. codegen.  So, it turns out we both do, and don't, parse these compound target-feature strings.

I'm leaning in the direction of having clang just support the compound target-feature strings, but I haven't found the right place for that just yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136106/new/

https://reviews.llvm.org/D136106