[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 14 15:17:29 PDT 2023


MaskRay added a comment.

In D152279#4420312 <https://reviews.llvm.org/D152279#4420312>, @asb wrote:

> In D152279#4415974 <https://reviews.llvm.org/D152279#4415974>, @MaskRay wrote:
>
>> However, RISC-V `-msmall-data-limit=` is probably a case warranting a difference.
>> The global pointer relaxation has a very limited value (benchmarked by multiple parties, including a party which implemented this feature in the GNU toolchain: even they can only say the optimization only applies to very specific projects).
>> The default value is confusing: as explained by the summary.
>>
>> I suspect that `-msmall-data-limit=8` is too conservative, maybe 16 would be better, I don't know. I think global pointer relaxation users should toggle this by themselves, not relying on `0` or `8` default decided by a bunch of strange conditions.
>
> I don't disagree that the small data limit being 8 rather than something else doesn't seem to be particularly well motivated, but I understand that the case where the option does make a difference is on embedded targets (I think the data that was shared before was for SPEC, but could be wrong?). I think we can generally expect more willingness for people targeting embedded systems to explore different compiler flags,

Agree.

> but just matching gcc does feel like a better default. What do you think about keeping the default for bare metal targets?

I am unsure we want to add the complexity and refrain from changing the default for bare metal targets due to haunted graveyards https://www.usenix.org/sites/default/files/conference/protected-files/srecon17americas_slides_reese.pdf

We are already different from GCC in a number of ways:

- for `-fpie`, we may emit `.sdata`/`.sbss` but GCC won't
- we accept `-G` while GCC doesn't.
- we don't emit `.srodata`.

I made a comment on https://lists.riscv.org/g/sig-toolchains/message/619 and informed GCC folks in case they can change the default as well.
If they don't, I think we made a good choice for not following this particular behavior.

(I feel sad that I did not see D57497 <https://reviews.llvm.org/D57497> and it landed with a blocked review. If we started from scratch, we would probably run into a cleaner state.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152279



More information about the cfe-commits mailing list