Re: [Valgrind-developers] vex: r3300 - /trunk/priv/guest_mips_toIR.c

[<pa...@fr...>]

Hi

I'm getting build breakage with this change.

----- Original Message -----
> Author: petarj
> Date: Fri Feb 10 17:58:40 2017
> New Revision: 3300
> 
> Log:
> mips: rewrite mips_irgen_load_and_add32|64 and code around it
> 
> Make sure that mips_irgen_load_and_add32 gets both expected value and
> new value, so the function code makes more sense and does load/store
> in
> a atomic way.
> 
> Minor renaming and code style issues added too.
> 
> Patch by Tamara Vlahovic.
> 
> Modified:
>     trunk/priv/guest_mips_toIR.c
> 
> Modified: trunk/priv/guest_mips_toIR.c
> ==============================================================================
> --- trunk/priv/guest_mips_toIR.c (original)
> +++ trunk/priv/guest_mips_toIR.c Fri Feb 10 17:58:40 2017
> @@ -2187,17 +2187,19 @@
>  }
>  
>  /* Based on s390_irgen_load_and_add32. */
> -static void mips_irgen_load_and_add32(IRTemp op1addr, IRTemp
> new_val,
> -                                      UChar rd, Bool putIntoRd)
> +static void mips_load_store32(IRTemp op1addr, IRTemp new_val,
> +                              IRTemp expd, UChar rd, Bool putIntoRd)
>  {
>     IRCAS *cas;
>     IRTemp old_mem = newTemp(Ity_I32);
> -   IRTemp expd    = newTemp(Ity_I32);
> -
> -   assign(expd, load(Ity_I32, mkexpr(op1addr)));
> +   IRType ty      = mode64 ? Ity_I64 : Ity_I32;
>  
>     cas = mkIRCAS(IRTemp_INVALID, old_mem,
> -                 Iend_LE, mkexpr(op1addr),
> +#if defined (_MIPSEL)
> +               Iend_LE, mkexpr(op1addr),
> +#elif defined (_MIPSEB)
> +               Iend_BE, mkexpr(op1addr),
> +#endif


As far as I can see, _MIPSEL and _MIPSEB and gcc builtin macros for the MIPS platform. The problem is that neither is defined on other platforms. This means that neither #if nor #elif is true, which causes the two arguments to me missing (see below).

Shouldn't this be

#if defined (_MIPSEL)
               Iend_LE, mkexpr(op1addr),
#else
               Iend_BE, mkexpr(op1addr),
#endif

(perhaps the condition the other way round).

A+
Paul


gcc -DHAVE_CONFIG_H -I. -I..  -I.. -I../include -I../VEX/pub -I../VEX/pub -DVGA_amd64=1 -DVGO_linux=1 -DVGP_amd64_linux=1 -DVGPV_amd64_linux_vanilla=1 -Ipriv  -m64 -O2 -g -std=gnu99 -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-declarations -Wcast-align -Wcast-qual -Wwrite-strings -Wempty-body -Wformat -Wformat-security -Wignored-qualifiers -Wmissing-parameter-type -Wold-style-declaration -fno-stack-protector -fno-strict-aliasing -fno-builtin  -fomit-frame-pointer -Wbad-function-cast -fstrict-aliasing  -MT priv/libvex_amd64_linux_a-guest_mips_toIR.o -MD -MP -MF priv/.deps/libvex_amd64_linux_a-guest_mips_toIR.Tpo -c -o priv/libvex_amd64_linux_a-guest_mips_toIR.o `test -f 'priv/guest_mips_toIR.c' || echo './'`priv/guest_mips_toIR.c
In file included from priv/guest_mips_toIR.c:39:0:
priv/guest_mips_toIR.c: In function ‘mips_load_store32’:
priv/main_util.h:44:14: error: incompatible type for argument 3 of ‘mkIRCAS’
 #define NULL ((void*)0)
              ^
priv/guest_mips_toIR.c:2203:18: note: in expansion of macro ‘NULL’
                  NULL, mkexpr(expd), /* expected value */
                  ^~~~
In file included from priv/guest_mips_toIR.c:34:0:
../VEX/pub/libvex_ir.h:2584:15: note: expected ‘IREndness {aka enum <anonymous>}’ but argument is of type ‘void *’
 extern IRCAS* mkIRCAS ( IRTemp oldHi, IRTemp oldLo,
               ^~~~~~~
priv/guest_mips_toIR.c:2197:10: error: too few arguments to function ‘mkIRCAS’
    cas = mkIRCAS(IRTemp_INVALID, old_mem,
          ^~~~~~~
In file included from priv/guest_mips_toIR.c:34:0:
../VEX/pub/libvex_ir.h:2584:15: note: declared here
 extern IRCAS* mkIRCAS ( IRTemp oldHi, IRTemp oldLo,