[llvm-commits] [llvm] r43176 - in /llvm/trunk/lib/Target/ARM: ARMISelLowering.cpp ARMISelLowering.h

[Rafael Espindola]

On 22/10/2007, Evan Cheng <evan.cheng at apple.com> wrote:
> Hi Rafael,
>
> There a couple of problems with this patch.
>
> 1. This breaks a number of tests for me:
> llc-beta /MultiSource/Applications/JM/ldecod/ldecod
> llc /MultiSource/Applications/JM/ldecod/ldecod
> llc /External/SPEC/CINT2000/254.gap/254.gap
> llc /MultiSource/Benchmarks/MiBench/office-ispell/office-ispell
> llc-beta /MultiSource/Benchmarks/MiBench/office-ispell/office-ispell
> llc-beta /External/SPEC/CINT2000/254.gap/254.gap

Sorry about that. I don't have an arm board with me. I will try to set
up a qemu chroot this Friday to better test on ARM.


> LowerMEMCPYInline() is asserting when the memcpy is 4-byte aligned but
> the size isn't multiple of 4. I'm going to remove the assertion and
> issue byte / word loads / stores to handle the trailing bytes. Please
> let me know if that's undesirable.

I now notice that the original code had a "I->getValue() & 3" then
call memcpy. You can just add a

if (Size & 3)
  LowerMEMCPYCall(...)

This will return to the previous behavior. Making the inline version
handle other sizes is also fine.

What I am trying to do is factor a bit of the memcpy expansion code
among the various backends since I will need it on all of them to
implement "byval" arguments.

> 2. The stores are unnecessarily serialized. Each of them should use
> the load tokenfactor as input chain. All the chains produced by the
> stores should then be fed into a new tokenfactor. I'll fix this.

The code that generates the inline expansion is the same as before.
Just on a different function. Any improvement is welcome :-)

> Thanks,
>
> Evan

Sorry for the problem...,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047