> -----Original Message----- > From: Tom Stellard [mailto:tom at stellard.net] > Sent: 24 November 2014 17:15 > To: Daniel Sanders > Cc: LLVM Developers Mailing List (llvmdev at cs.uiuc.edu) > Subject: Re: Proposed patches for Clang 3.5.1 > > On Mon, Nov 24, 2014 at 04:33:28PM +0000, Daniel Sanders wrote: > > Hi, > > > > I'd like to propose the following patches for inclusion in Clang 3.5.1. > > > > Proposed clang patches: > > > > * r213769 - Fix test/Driver/cl-x86-flags.c by providing explicit -target > > > > This one seems OK, but I would feel better if the X86 code owner approved > it too. Could you be pull this request into a separate mail and cc him.Ok.> > * r214025 - [Driver][Mips] Check output of -dynamic-linker arguments > by the Clang driver > > > > * r214662 - [Mips] Add the `mips64-linux-gnu` target to the test case to > check `in128` type handling. > > > > * r217147 - [mips] Zero-sized structs cannot be ignored in > MipsABIInfo::classifyReturnType() for O32 > > These look OK to me you you can merge them to the 3.5 branch yourself, > or if you aren't comfortable with this, I can do it. If you decide > to merge them yourself, make sure you use the merge script so we get a > consistent commit message format: utils/release/merge.sh > > Note this only works with SVN. If you use git, you may need to manually > paste > the svn commit log.Thanks. I'm happy to merge them.> > Proposed llvm patches: > > > > * r216920 - Fix left shifts of negative values in MipsDisassembler. > > > > * r221408 - [mips64] Fix MIPS64 exception personality encoding > > > > * r221453 - [mips] Tolerate the use of the %z inline asm operand > modifier with non-immediates. > > > > * r216262 - [mips] Don't use odd-numbered float registers for double > arguments for fastcc calling convention if FP is 64-bit and +nooddspreg is > used. > > > > This all look OK to me, go ahead an merge.Thanks.> > * r217257 - [mips] Change Feature-related types from unsigned to > uint64_t in MipsAsmParser. No functional changes. > > > > I'm a little concerned this might break the shared library ABI. We will need > to verify that it doesn't before it gets merged.I don't think this will break it since the two functions that are changed are private to MipsAsmParser and our Feature* constants in MipsGenSubtargetInfo.inc are already uint64_t. However, I haven't supported a shared library ABI before so I can't be sure. How would I check?> > * r218745 - [mips] Fix disassembly of [ls][wd]c[23], cache, and pref > > > > This looks OK to me.Thanks.> > I'd also like to propose the inclusion of the recent ABI fixes to the Mips > target but I'm not sure this is a good idea. I'm having difficulty sorting out the > dependencies for these at the moment since they seem to depend on some > of Eric Christopher's Subtarget/TargetMachine refactoring. It may also be a > bit large for a stable release since it's ~50 LLVM patches and ~8 Clang patches > and refactors a large amount of the Mips calling convention code. What do > you think? > > Can you give me an idea of how important these fixes are? My personal > feeling is that big fixes like this can sometimes be OK as long as they > are mostly contained to a specific target and that target isn't X86 > or ARM.Without them, the calling convention for big-endian 64-bit Mips targets is very badly broken. Clang is generally consistent with itself, but interlinking with GCC-compiled code doesn't work. When I started on this work ~4500 of 5000 tests generated by ABITestGen.py produced incorrect output when mixing GCC and Clang objects. 32-bit and little-endian Mips targets are ok except for a couple rare corner cases (e.g. 128-bit float return values). Known problems include: * Inreg struct arguments (and return values) of <64-bits are passed in the least significant bits of a register instead of the most significant * Arguments of <64-bits that are passed via the stack are in the wrong portion of the stack slot. * 128-bit floats are returned in the wrong registers. This one is actually a long-standing GCC bug that has become the de-facto standard. * Small structures in varargs are read from the wrong part of the stack slot. This doesn't work for GCC either. * Floating point arguments are passed correctly, but fixing some of the above bugs can easily break soft-float With these patches, big-endian 64-bit Mips targets successfully interlinks with GCC for almost everything I've tried. So far I've tested the first 100,000 ABITestGen.py tests for the N32 ABI (64-bit with 32-bit pointers), 10,000 for the N64 ABI (64-bit), and various varargs tests for both ABI's. The one known failure is small structures in vararg functions and this turns out to be broken on GCC too.> In this case, it would help too if you could provide some release > testing for MIPS.I'll be testing natively for MIPS32, and MIPS32r2 (both endians). I'll also be testing cross-compilation for MIPS64r2 (both endians) and MIPS64r6 (both endians).> Are you planning on back-porting these patches to 3.5 for personal use, > even if they aren't going to be included in the official release?If one of our customers requires it, yes. Otherwise, I'll stick to official releases. Sorry for the vague answer on this one.> -Tom > > > Daniel Sanders > > Leading Software Design Engineer, MIPS Processor IP > > Imagination Technologies Limited > > www.imgtec.com<http://www.imgtec.com/> > >
On Mon Nov 24 2014 at 12:23:57 PM Daniel Sanders <Daniel.Sanders at imgtec.com> wrote:> > -----Original Message----- > > From: Tom Stellard [mailto:tom at stellard.net] > > Sent: 24 November 2014 17:15 > > To: Daniel Sanders > > Cc: LLVM Developers Mailing List (llvmdev at cs.uiuc.edu) > > Subject: Re: Proposed patches for Clang 3.5.1 > > > > On Mon, Nov 24, 2014 at 04:33:28PM +0000, Daniel Sanders wrote: > > > Hi, > > > > > > I'd like to propose the following patches for inclusion in Clang 3.5.1. > > > > > > Proposed clang patches: > > > > > > * r213769 - Fix test/Driver/cl-x86-flags.c by providing > explicit -target > > > > > > > This one seems OK, but I would feel better if the X86 code owner approved > > it too. Could you be pull this request into a separate mail and cc him. > > Ok. > > > > * r214025 - [Driver][Mips] Check output of -dynamic-linker > arguments > > by the Clang driver > > > > > > * r214662 - [Mips] Add the `mips64-linux-gnu` target to the > test case to > > check `in128` type handling. > > > > > > * r217147 - [mips] Zero-sized structs cannot be ignored in > > MipsABIInfo::classifyReturnType() for O32 > > > > These look OK to me you you can merge them to the 3.5 branch yourself, > > or if you aren't comfortable with this, I can do it. If you decide > > to merge them yourself, make sure you use the merge script so we get a > > consistent commit message format: utils/release/merge.sh > > > > Note this only works with SVN. If you use git, you may need to manually > > paste > > the svn commit log. > > Thanks. I'm happy to merge them. > > > > Proposed llvm patches: > > > > > > * r216920 - Fix left shifts of negative values in > MipsDisassembler. > > > > > > * r221408 - [mips64] Fix MIPS64 exception personality encoding > > > > > > * r221453 - [mips] Tolerate the use of the %z inline asm > operand > > modifier with non-immediates. > > > > > > * r216262 - [mips] Don't use odd-numbered float registers for > double > > arguments for fastcc calling convention if FP is 64-bit and +nooddspreg > is > > used. > > > > > > > This all look OK to me, go ahead an merge. > > Thanks. > > > > * r217257 - [mips] Change Feature-related types from unsigned > to > > uint64_t in MipsAsmParser. No functional changes. > > > > > > > I'm a little concerned this might break the shared library ABI. We will > need > > to verify that it doesn't before it gets merged. > > I don't think this will break it since the two functions that are changed > are private to MipsAsmParser and our Feature* constants in > MipsGenSubtargetInfo.inc are already uint64_t. However, I haven't supported > a shared library ABI before so I can't be sure. How would I check? > > > > * r218745 - [mips] Fix disassembly of [ls][wd]c[23], cache, > and pref > > > > > > > This looks OK to me. > > Thanks. > > > > I'd also like to propose the inclusion of the recent ABI fixes to the > Mips > > target but I'm not sure this is a good idea. I'm having difficulty > sorting out the > > dependencies for these at the moment since they seem to depend on some > > of Eric Christopher's Subtarget/TargetMachine refactoring. It may also > be a > > bit large for a stable release since it's ~50 LLVM patches and ~8 Clang > patches > > and refactors a large amount of the Mips calling convention code. What do > > you think? > > > > Can you give me an idea of how important these fixes are? My personal > > feeling is that big fixes like this can sometimes be OK as long as they > > are mostly contained to a specific target and that target isn't X86 > > or ARM. > > Without them, the calling convention for big-endian 64-bit Mips targets is > very badly broken. Clang is generally consistent with itself, but > interlinking with GCC-compiled code doesn't work. When I started on this > work ~4500 of 5000 tests generated by ABITestGen.py produced incorrect > output when mixing GCC and Clang objects. 32-bit and little-endian Mips > targets are ok except for a couple rare corner cases (e.g. 128-bit float > return values). > >It's likely possible to rewrite the patches so that they don't depend on my other work. If you have any questions there or need me to stamp the rewrites as "what should happen if you don't take my patches into account" I'm happy to do so. Thanks. -eric> Known problems include: > * Inreg struct arguments (and return values) of <64-bits are passed in the > least significant bits of a register instead of the most significant > * Arguments of <64-bits that are passed via the stack are in the wrong > portion of the stack slot. > * 128-bit floats are returned in the wrong registers. This one is actually > a long-standing GCC bug that has become the de-facto standard. > * Small structures in varargs are read from the wrong part of the stack > slot. This doesn't work for GCC either. > * Floating point arguments are passed correctly, but fixing some of the > above bugs can easily break soft-float > > With these patches, big-endian 64-bit Mips targets successfully interlinks > with GCC for almost everything I've tried. So far I've tested the first > 100,000 ABITestGen.py tests for the N32 ABI (64-bit with 32-bit pointers), > 10,000 for the N64 ABI (64-bit), and various varargs tests for both ABI's. > The one known failure is small structures in vararg functions and this > turns out to be broken on GCC too. > > > In this case, it would help too if you could provide some release > > testing for MIPS. > > I'll be testing natively for MIPS32, and MIPS32r2 (both endians). I'll > also be testing cross-compilation for MIPS64r2 (both endians) and MIPS64r6 > (both endians). > > > Are you planning on back-porting these patches to 3.5 for personal use, > > even if they aren't going to be included in the official release? > > If one of our customers requires it, yes. Otherwise, I'll stick to > official releases. Sorry for the vague answer on this one. > > > -Tom > > > > > Daniel Sanders > > > Leading Software Design Engineer, MIPS Processor IP > > > Imagination Technologies Limited > > > www.imgtec.com<http://www.imgtec.com/> > > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141125/ec859cf7/attachment.html>
On 24 Nov 2014, at 20:05, Daniel Sanders <Daniel.Sanders at imgtec.com> wrote:> Clang is generally consistent with itself, but interlinking with GCC-compiled code doesn't workActually, clang is pretty inconsistent with itself on bit-endian 64-bit MIPS prior to this work. Any arguments that end up on the stack are corrupted when calling from clang-compiled code to clang-compiled code. This is most obvious in variadic calls, but breaks in a load of other places too. David
> > > > I'd also like to propose the inclusion of the recent ABI fixes to the Mips > > > > target but I'm not sure this is a good idea. I'm having difficulty sorting out the > > > > dependencies for these at the moment since they seem to depend on some > > > > of Eric Christopher's Subtarget/TargetMachine refactoring. It may also be a > > > > bit large for a stable release since it's ~50 LLVM patches and ~8 Clang patches > > > > and refactors a large amount of the Mips calling convention code. What do > > > > you think? > > > > > > Can you give me an idea of how important these fixes are? My personal > > > feeling is that big fixes like this can sometimes be OK as long as they > > > are mostly contained to a specific target and that target isn't X86 > > > or ARM. > > > > Without them, the calling convention for big-endian 64-bit Mips targets is very badly broken. Clang is generally consistent with itself, > > but interlinking with GCC-compiled code doesn't work. When I started on this work ~4500 of 5000 tests generated by ABITestGen.py > > produced incorrect output when mixing > GCC and Clang objects. 32-bit and little-endian Mips targets are ok except for a couple rare > > corner cases (e.g. 128-bit float return values). > > It's likely possible to rewrite the patches so that they don't depend on my other work. If you have any questions there or need me to stamp the rewrites as "what should happen if you don't take my patches into account" I'm happy to do so. > > Thanks. > > -ericI believe I've managed to backport them to the release_35 branch but I haven't re-tested them beyond running 'ninja check-all' yet. The patches are currently at https://github.com/dsandersimgtec/clang/commits/release_35, and https://github.com/dsandersimgtec/llvm/commits/release_35_proposed. I'm going to run as much ABI testing as possible on it this evening. _______________________________________________ LLVM Developers mailing list LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Ah yes, that's right. I thought it was at least consistent enough to pass the LLVM test-suite but I'm mistaken. At the time this was first reported (June/July), it turned out that there was no big-endian MIPS64 builder in our internal buildbot. The one we set up when we noticed this had 12 test-suite failures.> -----Original Message----- > From: Dr D. Chisnall [mailto:dc552 at hermes.cam.ac.uk] On Behalf Of David > Chisnall > Sent: 25 November 2014 09:33 > To: Daniel Sanders > Cc: Tom Stellard; LLVM Developers Mailing List (llvmdev at cs.uiuc.edu) > Subject: Re: [LLVMdev] Proposed patches for Clang 3.5.1 > > On 24 Nov 2014, at 20:05, Daniel Sanders <Daniel.Sanders at imgtec.com> > wrote: > > > Clang is generally consistent with itself, but interlinking with GCC-compiled > code doesn't work > > Actually, clang is pretty inconsistent with itself on bit-endian 64-bit MIPS prior > to this work. Any arguments that end up on the stack are corrupted when > calling from clang-compiled code to clang-compiled code. This is most > obvious in variadic calls, but breaks in a load of other places too. > > David