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

2023-10-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 557659. MaskRay added a comment. test rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152279/new/ https://reviews.llvm.org/D152279 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/RISCV

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

2023-09-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152279#4638173 , @asb wrote: > In D152279#4612099 , @craig.topper > wrote: > >> In D152279#4612087 , @MaskRay >> wrote: >> >>> I am still in

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

2023-09-05 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment. In D152279#4612099 , @craig.topper wrote: > In D152279#4612087 , @MaskRay wrote: > >> I am still interested in moving this forward. What should be done here? If >> the decision is to keep th

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152279#4612099 , @craig.topper wrote: > In D152279#4612087 , @MaskRay wrote: > >> I am still interested in moving this forward. What should be done here? If >> the decision is to kee

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

2023-08-23 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment. In D152279#4612087 , @MaskRay wrote: > I am still interested in moving this forward. What should be done here? If > the decision is to keep the current odd default 8 for > `toolchains::RISCVToolChain`, I guess I'll have to

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

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I am still interested in moving this forward. What should be done here? If the decision is to keep the current odd default 8 for `toolchains::RISCVToolChain`, I guess I'll have to take the compromise as making a step forward is better than nothing. Repository: rG LL

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

2023-06-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Thank you for chiming in. I disagree that we keep default=8 for embedded without understanding (a) why 8 provides values and (b) justifying that the value is significant enough for embedded to be different. I think Alex's argument "I think we can generally expect more w

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

2023-06-14 Thread Shiva Chen via Phabricator via cfe-commits
shiva0217 added a comment. In D152279#4422887 , @MaskRay wrote: > In D152279#4420312 , @asb wrote: > >> In D152279#4415974 , @MaskRay >> wrote: >> >>> However, RISC-V `-m

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

2023-06-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152279#4420312 , @asb wrote: > In D152279#4415974 , @MaskRay wrote: > >> However, RISC-V `-msmall-data-limit=` is probably a case warranting a >> difference. >> The global pointer rel

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

2023-06-14 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment. In 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 part

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

2023-06-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152279#4414574 , @hiraditya wrote: > In D152279#4406896 , @MaskRay wrote: > >> In D152279#4405901 , @asb wrote: >> >>> One of the key things w

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

2023-06-12 Thread Aditya Kumar via Phabricator via cfe-commits
hiraditya added a comment. In D152279#4406896 , @MaskRay wrote: > In D152279#4405901 , @asb wrote: > >> One of the key things we've been discussing on this at the LLVM call is that >> we probably want to keep the

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

2023-06-08 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D152279#4405901 , @asb wrote: > One of the key things we've been discussing on this at the LLVM call is that > we probably want to keep the small data limit for embedded targets. It'd be useful to hear from some concrete embe

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

2023-06-08 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment. One of the key things we've been discussing on this at the LLVM call is that we probably want to keep the small data limit for embedded targets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152279/new/ https://reviews.llvm.or

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

2023-06-07 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment. In D152279#4403940 , @phosek wrote: > We're planning to default to `-msmall-data-limit=0` for Android and Fuchsia > so I'm supportive of this change because it means less complexity and fewer > differences between platforms. > > I t

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

2023-06-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. We're planning to default to `-msmall-data-limit=0` for Android and Fuchsia so I'm supportive of this change because it means less complexity and fewer differences between platforms. I think it would be worth bringing up this topic at the RISC-V LLVM sync-up call tomorr

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

2023-06-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2087 // Not support linker relaxation for PIC. -SmallDataLimit = "0"; if (Args.hasArg(options::OPT_G)) { D.Diag(diag::warn_drv_unsupported_sdata); The code is wr

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

2023-06-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: apazos, asb, craig.topper, hiraditya, jrtc27, shiva0217. Herald added subscribers: luke, frasercrmck, luismarques, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, niosHD, sabu