llvm dev - May 2015 - [LLVMdev] 3.6.1 -rc1 has been tagged. Testing begins.

> I've disassembled the failing
MultiSource/Benchmarks/tramp3d-v4/tramp3d-v4 and compared it to
> the one from the LLVM 3.6.0 test runs. There's nothing obvious.
We've removed some useless
> 'addiu $sp,$sp,0', eliminated two (seemingly redundant) sign
extends, and the addresses of
> functions+data has changed slightly.
I've investigated further and I'm fairly confident that r235869
(http://llvm.org/viewvc/llvm-project?view=revision&revision=235869) is the
cause of this regression.

The problem is these three definitions:
  // Bypass trunc nodes for bitwise ops.
  def : MipsPat<(i32 (trunc (and GPR64:$lhs, GPR64:$rhs))),
                (EXTRACT_SUBREG (AND64 GPR64:$lhs, GPR64:$rhs), sub_32)>;
  def : MipsPat<(i32 (trunc (or GPR64:$lhs, GPR64:$rhs))),
                (EXTRACT_SUBREG (OR64 GPR64:$lhs, GPR64:$rhs), sub_32)>;
  def : MipsPat<(i32 (trunc (xor GPR64:$lhs, GPR64:$rhs))),
                (EXTRACT_SUBREG (XOR64 GPR64:$lhs, GPR64:$rhs), sub_32)>;
They're correct around 95% of the time since the instructions that act on
i32 only care about the lowest 32-bits of the GPR. However, comparison
instructions such as BEQ compare the whole 64-bit GPR, even for i32, and
therefore needed the sign-extend we used to generate.

I'm going to try a build with that patch reverted to confirm this and
assuming I'm right I'll revert this merge.
> clang+llvm-3.6.1-rc1-x86_64-linux-gnu-ubuntu-14.04.tar.xz (cross compiling
to Mips)
>         * Still running.
>         * For the 'mipsel-img-linux-gnu -mips32r6 -mfpxx',
'mipsel-img-linux-gnu -mmicromips' test runs I have
>            several regressions that look like timeouts caused by a busy
system. I'm re-running these at the
>            moment and they are passing so far.
Just a quick update: The 'mipsel-img-linux-gnu -mips32r6 -mfpxx'
failures all passed when rerun while the system was less busy. I've just
started the re-runs for 'mipsel-img-linux-gnu -mmicromips'.

________________________________________
From: Daniel Sanders
Sent: 14 May 2015 11:52
To: Tom Stellard; llvmdev at cs.uiuc.edu
Cc: cfe-dev at cs.uiuc.edu
Subject: RE: [LLVMdev] 3.6.1 -rc1 has been tagged.  Testing begins.

clang+llvm-3.6.1-rc1-mips-linux-gnu.tar.xz
        All good

clang+llvm-3.6.1-rc1-mipsel-linux-gnu.tar.xz
        Still running.
        So far only ClamAV has failed. This seems to be due to forgetting to
install zlib1g-dev after rebuilding the system recently. I'll re-run this
test once the others finish

clang+llvm-3.6.1-rc1-x86_64-linux-gnu-ubuntu-14.04.tar.xz (cross compiling to
Mips)
        * Still running.
        * For the 'mipsel-img-linux-gnu -mips32r6 -mfpxx',
'mipsel-img-linux-gnu -mmicromips' test runs I have several regressions
that look like timeouts caused by a busy system. I'm re-running these at the
moment and they are passing so far.
        * For 'mips-img-linux-gnu -mips64r6 -mabi=n32' test run I have
several regressions that look like timeouts caused by a busy system. I'll
re-run these once the rest finish.
        * For the 'mips-img-linux-gnu -mips64r6 -mabi=n32' test run I
also have two failures that look like regressions.
MultiSource/Benchmarks/tramp3d-v4/tramp3d-v4 is segfaulting.
MultiSource/Applications/kimwitu++/kc is failing the hashed-output reference
check. I'm looking into these.

I've disassembled the failing MultiSource/Benchmarks/tramp3d-v4/tramp3d-v4
and compared it to the one from the LLVM 3.6.0 test runs. There's nothing
obvious. We've removed some useless 'addiu $sp,$sp,0', eliminated
two (seemingly redundant) sign extends, and the addresses of functions+data has
changed slightly.
> -----Original Message-----
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu]
> On Behalf Of Tom Stellard
> Sent: 11 May 2015 21:30
> To: llvmdev at cs.uiuc.edu
> Cc: cfe-dev at cs.uiuc.edu
> Subject: [LLVMdev] 3.6.1 -rc1 has been tagged. Testing begins.
>
> Hi,
>
> I have tagged the 3.6.1-rc1 so testing can begin.  We can always use
> more testers, so if you are interested in helping, let me know.
>
> Instructions for validating an LLVM release can be found here:
> http://llvm.org/docs/ReleaseProcess.html
>
> Reminder: We are using 3.6.0 as our baseline for regression testing.
>
> Thanks,
> Tom
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On 14 May 2015 at 21:39, Daniel Sanders <Daniel.Sanders at imgtec.com>
wrote:
> They're correct around 95% of the time since the instructions that act
on i32 only care about the lowest 32-bits of the GPR. However, comparison
instructions such as BEQ compare the whole 64-bit GPR, even for i32, and
therefore needed the sign-extend we used to generate.
Has this been fixed in trunk? If the fix is simple, than maybe
applying it would be preferable to reverting this patch, no?

cheers,
--renato

> -----Original Message-----
> From: Renato Golin [mailto:renato.golin at linaro.org]
> Sent: 15 May 2015 12:51
> To: Daniel Sanders
> Cc: Tom Stellard; llvmdev at cs.uiuc.edu; cfe-dev at cs.uiuc.edu
> Subject: Re: [LLVMdev] 3.6.1 -rc1 has been tagged. Testing begins.
> 
> On 14 May 2015 at 21:39, Daniel Sanders <Daniel.Sanders at
imgtec.com>
> wrote:
> > They're correct around 95% of the time since the instructions that
act on i32
> only care about the lowest 32-bits of the GPR. However, comparison
> instructions such as BEQ compare the whole 64-bit GPR, even for i32, and
> therefore needed the sign-extend we used to generate.
> 
> Has this been fixed in trunk? If the fix is simple, than maybe
> applying it would be preferable to reverting this patch, no?
> 
> cheers,
> --renato
Not yet, these two failures were the first time we had seen the problem.
We're still looking into fixing it properly but I'm currently thinking
that the correct fix is to add
  if (Subtarget.isGP64bit())
    setOperationAction(ISD::SETCC, MVT::i32, Promote);
to MipsISelLowering.cpp and sort out the consequences of this on the patterns
for all the comparison instructions. This is likely to be a fairly big change to
our target.