RFC: [MIPS] Add an option to disable ldc1/sdc1
Hello All, Once in a while we got reports about programs (ex: WebKit, FireFox) crash due to ldc1/sdc1 unaligned accesses on MIPS targets. The root cause is that programmers neglect the alignment issue and cast arbitrary pointers to point to double variables. Although the correct solution is to fix application source code to fulfill alignment requirements, we want to add a GCC option to disable ldc1 and sdc1 (for the testing purpose or for workaround). On 32-bit MIPS targets, GCC generates lwc1 and swc1 when -mno-ldc1-sdc1 is used, so that the memory address can be just 4-byte aligned to avoid ldc1/sdc1 address exceptions. Ex: (The proposed patch) Index: mips.opt === --- mips.opt(revision 196026) +++ mips.opt(working copy) @@ -233,6 +233,10 @@ Target Report RejectNegative Mask(MIPS3D) Use MIPS-3D instructions +mldc1-sdc1 +Target Report Var(TARGET_LDC1_SDC1) Init(1) +Use ldc1 and sdc1 instruction + mllsc Target Report Mask(LLSC) Use ll, sc and sync instructions Index: mips.h === --- mips.h (revision 196026) +++ mips.h (working copy) @@ -840,8 +840,9 @@ ST Loongson 2E/2F. */ #define ISA_HAS_CONDMOVE(ISA_HAS_FP_CONDMOVE || TARGET_LOONGSON_2EF) -/* ISA has LDC1 and SDC1. */ -#define ISA_HAS_LDC1_SDC1 (!ISA_MIPS1 && !TARGET_MIPS16) +/* ISA has LDC1 and SDC1 and they are enabled. */ +#define ISA_HAS_LDC1_SDC1 \ + (!ISA_MIPS1 && !TARGET_MIPS16 && TARGET_LDC1_SDC1) /* ISA has the mips4 FP condition code instructions: FP-compare to CC, branch on CC, and move (both FP and non-FP) on CC. */ Any feedback? Thanks a lot! Regards, Chao-ying
RE: RFC: [MIPS] Add an option to disable ldc1/sdc1
Richard Sandiford wrote: > Chao-Ying Fu writes: > > Hello All, > > > > Once in a while we got reports about programs (ex: > WebKit, FireFox) > > crash due to ldc1/sdc1 unaligned accesses on MIPS targets. The root > > cause is that programmers neglect the alignment issue and cast > > arbitrary pointers to point to double variables. > > > > Although the correct solution is to fix application source code to > > fulfill alignment requirements, we want to add a GCC option > to disable > > ldc1 and sdc1 (for the testing purpose or for workaround). > On 32-bit > > MIPS targets, GCC generates lwc1 and swc1 when > -mno-ldc1-sdc1 is used, > > so that the memory address can be just 4-byte aligned to avoid > > ldc1/sdc1 address exceptions. > > Sounds OK to me, given that the impact of the option is so low. Thanks a lot for OK! > Bikeshed time, but I'd prefer the option to be named after the thing > that we're guaranteeing, rather than an instruction. E.g. if the > problem is that doubles might only be 32-bit aligned, we could have > -mmisaligned-double (better suggestions welcome). > What about 64-bit targets? We can sometimes access doubles > using GPRs, > so on 64-bit targets we could end up using LD and SD to > access a double > even when this option disables LDC1 and SDC1. I think we'd need to > patch the move patterns as well. > > If you only see the problem on 32-bit targets, then maybe it would be > better to have an option that prevents instructions that > require greater > than word alignment. Say (for sake of argument) -mno-superword-align. > Then it would be: > > #define ISA_HAS_LDC1_SDC1 \ > (!ISA_MIPS1 && !TARGET_MIPS16 && (TARGET_64BIT || > TARGET_SUPERWORD_ALIGN)) We only got reports about 32-bit MIPS targets. But similar issues may happen on 64-bit MIPS targets. Note that these problematic programs can run fine on other architectures that allow 8-byte load/store instructions from 4-byte aligned addresses. For a short-term goal, we can use -mon-superword-align (or other option names) to disable ldc1 and sdc1 on 32-bit MIPS targets where only ldc1 and sdc1 have this 8-byte alignment rule. Later, we can extend this option to cover 64-bit MIPS targets. Thanks! Regards, Chao-ying
RE: RFC: [MIPS] Add an option to disable ldc1/sdc1
Maciej W. Rozycki wrote: > > On Thu, 14 Feb 2013, Richard Sandiford wrote: > > > What about 64-bit targets? We can sometimes access doubles > using GPRs, > > so on 64-bit targets we could end up using LD and SD to > access a double > > even when this option disables LDC1 and SDC1. I think we'd need to > > patch the move patterns as well. > > As far as Linux is concerned -- which from the context of this > consideration I infer is the case -- there is no issue, > because unless an > app has explicitly requested this feature to be disabled, > with the use of > the sysmips(2) FIXADE API (which of course hardly any does), > all address > error exceptions triggered by an unaligned memory access made > with a CPU > instruction are trapped by the kernel and the access emulated. > > However no such accesses are emulated when made with a coprocessor > instruction, whether the FPU or CP2 or the legacy CP1 and CP3 > units as > supported by earlier processors. There was once a proposal > to emulate FPU > instructions as well, posted on the linux-mips mailing list here: > > http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=2012 0615234641.6938B58FE7C%40mail.viric.name > > -- but the MIPS/Linux maintainer rejected the idea. I have > cc-ed Ralf on > this reply in case he wanted to add something. The kernel emulation can help, but the performance is not great (as Ralf pointed out). So, this compiler workaround may have its advantages. > > Of course an option to GCC to disable any such instructions > may have some > value to some people -- for bare-iron targets if nothing else It is still good for 32-bit MIPS Linux targets. Right? > -- but I > fear this is going to end up with a lot of hassle with 64-bit > ABIs or even > just 64-bit FPU (-mabi=32 -mfp64) as individual halves of > 64-bit registers > are not addressable in the MIPS architecture (e.g. there's no LWHC1 > instruction), so you'll need to use scratch registers. Thanks for this point! I tested the patch and it cannot work with -mabi=32 -mfp64. Probably we can limit the scope of the patch to 32-bit MIPS targets with 32-bit FPU. > > Which is why I think any resources put into this effort > would better be > used to clean up such broken code and, perhaps more importantly, to > educate people to write their programs properly in the first place. > Writing portable code is really not a big deal, you just need > to remember > all the world is not x86 and apply a few simple rules. > I know this, but it's very hard to educate people and let people admit it's not CPU's fault, when same software can run on other architectures. Thanks a lot for your inputs! Regards, Chao-ying
Re: [EXTERNAL][PATCH 0/61] Improve Mips target
> This patch series improves the support for the mips64r6 target in GCC, > includes the enhancements to the general bug fixes and contains other > MIPS ISA and processor enablement. > > These patches are cherry-picked from the mips_rel/11_2_0/master > and mips_rel/9_3_0/master branches from the MIPS' repository: > https://github.com/MIPS/gcc . > Further details on the individual changes are included in the > respective patches. FYI. MIPS asked HTEC group to upstream our GCC patches to gcc.gnu.org. All patches should be covered by FSF copyright assignment from MIPS long time ago. If new documents/forms are needed, please let us know. Thanks a lot! Regards, Chao-ying
