[LLVMdev] Proposed patches for Clang 3.5.1

[Daniel Sanders]

> -----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/>
> >