Hi! On Thu, Oct 01, 2020 at 10:57:48PM +0930, Alan Modra wrote: > On Wed, Sep 30, 2020 at 03:56:32PM -0500, Segher Boessenkool wrote: > > On Wed, Sep 30, 2020 at 05:01:45PM +0930, Alan Modra wrote: > > > * config/rs6000/linux64.h (SUBSUBTARGET_OVERRIDE_OPTIONS): Don't > > > set -mcmodel=small for -mno-minimal-toc when pcrel. > > > > > - SET_CMODEL (CMODEL_SMALL); \ > > > + if (TARGET_MINIMAL_TOC \ > > > + || !(TARGET_PCREL \ > > > + || (PCREL_SUPPORTED_BY_OS \ > > > + && (rs6000_isa_flags_explicit \ > > > + & OPTION_MASK_PCREL) == 0))) \ > > > + SET_CMODEL (CMODEL_SMALL); \ > > > > Please write this in a more readable way? With some "else" statements, > > perhaps. > > > > It is also fine to SET_CMODEL twice if that makes for simpler code. > > Committed as per your suggestion.
Thanks. > I was looking at it again today > with the aim of converting this ugly macro to a function, and spotted > the duplication in freebsd64.h. Which has some bit-rot. > > Do you like the following? rs6000_linux64_override_options is > functionally the same as what was in linux64.h. I built various > configurations to test the change, powerpc64-linux, powerpc64le-linux > without any 32-bit targets enabled, powerpc64-freebsd12.0. Please do this as two patches? One the refactoring without any functional changes (which is pre-approved -- the name "linux64" isn't great if you use it in other OSes as well, but I cannot think of a better name either), and the other the actual change (which probably is fine as well, but it is hard to see with the patch like this). Thanks, Segher
