search

riscv-non-isa / riscv-c-api-doc

Documentation of the RISC-V C API
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
68 stars 38 forks source link

Mismatched base ISAs in target attribute arch #64

Closed lukel97 closed 8 months ago

lukel97 commented 8 months ago

I can't find anything in the doc that explicitly disallows specifying a target attribute with a different base ISA arch than the translation unit. E.g. with the current LLVM implementation, it's possible to compile

__attribute__((target("arch=rv32i")))
int foo(int x) {
  return x;
}

With clang -march=rv64i.

I presume it shouldn't be possible to mix base ISAs, and that an error should be raised in this case.

cmuellner commented 8 months ago

I would claim that only expert users (who know what they are doing) use the target attribute. Therefore, we should not implement any rules that disallow any combinations of the march string and the target attribute. But I agree that switching the base ISA is very unlikely.

lukel97 commented 8 months ago

So to make sure I'm understanding this correctly, it should be possible to mix base ISAs within the same translation unit? If that's the case then I think we would need to fix this in LLVM since we currently just ignore the base ISA

cmuellner commented 8 months ago

I'm not sure if someday there will be a use-case for mixing base ISAs in the same TU. So, in doubt, I would not forbid it here (unless GCC and LLVM maintainers want to do that explicitly).

This still allows more restrictions in the implementation. So, if a compiler does not want to support mixing bases, then it could trigger an error in this case. If that's not done right now, then this is a potential PR for the implementation. If the patch review discussion comes to the conclusion that we should do something in this repo, then we can follow up.

lukel97 commented 8 months ago

Thanks for clarifying. Happy to close this for now then.