[llvm] r295300 - [ARM] GlobalISel: Legalize 64-bit G_FADD and G_LOAD

[Quentin Colombet via llvm-commits]

Hi Diana,


> On Feb 16, 2017, at 1:09 AM, Diana Picus via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: rovka
> Date: Thu Feb 16 03:09:49 2017
> New Revision: 295300
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=295300&view=rev
> Log:
> [ARM] GlobalISel: Legalize 64-bit G_FADD and G_LOAD
> 
> For now we just mark them as legal all the time and let the other passes bail
> out if they can't handle it. In the future, we'll want to move more of the
> brains into the legalizer.
> 
> Modified:
>    llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp
>    llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-legalizer.mir
> 
> Modified: llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp?rev=295300&r1=295299&r2=295300&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMLegalizerInfo.cpp Thu Feb 16 03:09:49 2017
> @@ -32,6 +32,7 @@ ARMLegalizerInfo::ARMLegalizerInfo() {
>   const LLT s8 = LLT::scalar(8);
>   const LLT s16 = LLT::scalar(16);
>   const LLT s32 = LLT::scalar(32);
> +  const LLT s64 = LLT::scalar(64);
> 
>   setAction({G_FRAME_INDEX, p0}, Legal);
> 
> @@ -39,6 +40,11 @@ ARMLegalizerInfo::ARMLegalizerInfo() {
>     setAction({G_LOAD, Ty}, Legal);
>   setAction({G_LOAD, 1, p0}, Legal);
> 
> +  // FIXME: This is strictly for loading double-precision floating point values,
> +  // if the hardware allows it. We rely on the instruction selector to complain
> +  // otherwise.

I’m kind of puzzled by this fixme. If you can load 64-bit value, why would you care about marking this illegal?

My understanding is that this should be illegal only because of alignment constraint not match on the load or the absence of VFP (or whatever :)) extension.

What is your plan when you say “rely on instruction selector to complain”?

Cheers,
-Quentin

> +  setAction({G_LOAD, s64}, Legal);
> +
>   for (auto Ty : {s1, s8, s16, s32})
>     setAction({G_ADD, Ty}, Legal);
> 
> @@ -51,6 +57,7 @@ ARMLegalizerInfo::ARMLegalizerInfo() {
>   // FIXME: This is a bit sloppy, but for now we'll just rely on the instruction
>   // selector to complain if it doesn't support floating point.
>   setAction({G_FADD, s32}, Legal);
> +  setAction({G_FADD, s64}, Legal);
> 
>   computeTables();
> }
> 
> Modified: llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-legalizer.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-legalizer.mir?rev=295300&r1=295299&r2=295300&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-legalizer.mir (original)
> +++ llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-legalizer.mir Thu Feb 16 03:09:49 2017
> @@ -11,6 +11,7 @@
>   define void @test_legal_loads() { ret void }
> 
>   define void @test_fadd_s32() { ret void }
> +  define void @test_fadd_s64() { ret void }
> ...
> ---
> name:            test_sext_s8
> @@ -174,11 +175,13 @@ registers:
>   - { id: 3, class: _ }
>   - { id: 4, class: _ }
>   - { id: 5, class: _ }
> +  - { id: 6, class: _ }
> body:             |
>   bb.0:
>     liveins: %r0, %r1, %r2, %r3
> 
>     ; These are all legal, so we should find them unchanged in the output
> +    ; CHECK-DAG: {{%[0-9]+}}(s64) = G_LOAD %0
>     ; CHECK-DAG: {{%[0-9]+}}(s32) = G_LOAD %0
>     ; CHECK-DAG: {{%[0-9]+}}(s16) = G_LOAD %0
>     ; CHECK-DAG: {{%[0-9]+}}(s8) = G_LOAD %0
> @@ -190,6 +193,7 @@ body:             |
>     %3(s8)  = G_LOAD %0(p0)
>     %4(s1)  = G_LOAD %0(p0)
>     %5(p0)  = G_LOAD %0(p0)
> +    %6(s64) = G_LOAD %0(p0)
>     BX_RET 14, _
> ...
> ---
> @@ -217,3 +221,28 @@ body:             |
>     BX_RET 14, _, implicit %r0
> 
> ...
> +---
> +name:            test_fadd_s64
> +# CHECK-LABEL: name: test_fadd_s64
> +legalized:       false
> +# CHECK: legalized: true
> +regBankSelected: false
> +selected:        false
> +tracksRegLiveness: true
> +registers:
> +  - { id: 0, class: _ }
> +  - { id: 1, class: _ }
> +  - { id: 2, class: _ }
> +body:             |
> +  bb.0:
> +    liveins: %d0, %d1
> +
> +    %0(s64) = COPY %d0
> +    %1(s64) = COPY %d1
> +    %2(s64) = G_FADD %0, %1
> +    ; G_FADD with s64 is legal, so we should find it unchanged in the output
> +    ; CHECK: {{%[0-9]+}}(s64) = G_FADD {{%[0-9]+, %[0-9]+}}
> +    %d0 = COPY %2(s64)
> +    BX_RET 14, _, implicit %d0
> +
> +...
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits