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

Shiva Chen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 14 20:38:39 PDT 2023


shiva0217 added a comment.

In D152279#4422887 <https://reviews.llvm.org/D152279#4422887>, @MaskRay wrote:

> 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.)

Hi MaskRay,

-fpie is an oversight. Thanks for pointing out. For D57497 <https://reviews.llvm.org/D57497>, I thought it behave as good community citizen that trying to address all the comments.
Is there unaddressing comment? If there is, that might not be intended.

The patch tries to mimic the GCC to enable the relaxation. It generally put the data smaller than the threshold to the small data section which near gp and has higher possibility to transfer to gp based load/store.

Relaxation is less useful for large scale software. In previous time, most developers focus on embedded worlds. Now the flags become less friendly for the OS world.
I agree to change the default to make the OS world life easier.

I also agree with Alex. Should we preserve the threshold for embedded usage? Should we try to sync with GCC as possible?


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