|
From: John R. <jr...@bi...> - 2023-04-19 15:08:19
|
On 4/18/23 12:53, Carl Love via Valgrind-developers wrote:
> Mark:
>
> On Mon, 2023-04-17 at 09:22 -0700, Carl Love via Valgrind-developers
> wrote:
>> The test_isa_3_1_R1_RT and test_isa_3_1_R1_XT tests seem to run
>> differently then expected. The tests generate multiple lines of
>> output
>> when only one line was expected. For example:
>
> I have pushed a fix for the two tests. The issue is the tests are
> testing load instructions that load relative to the current PC address.
> The tests of these instructions adds blocks of OR immediate
> instructions before the assembly for the instruction under test.
> Unfortunately, the test didn't save and restore the registers touched
> by the OR immediate instructions. This is fine as long as you are
> calling a function, the touched registers are volitile across a
> function call. It seems the more recent GCC is a bit more aggressive
> in inlining the test functions. However, the compiler doesn't realize
> that the inline OR immediate instructions are touching registers. The
> OR immediate instructions were inadvertently changing the value of the
> register that held the for loop variable. Thus the loops would execute
> more times then expected.
>
> The commit is:
>
> commit 20cc0680c3491e062c76605b24e76dc02e16ef47 (HEAD -> master)
> Author: Carl Love <ce...@us...>
> Date: Mon Apr 17 17:12:25 2023 -0400
>
> PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c
>
> If the commit gets into the current release, great. If not, it is not
> an issue. The problem is completely isolated to the test case.
At the lowest level, the problem is that a statement such as
__asm__ __volatile__ ("ori 21,21,21");
in the PAD_ORI macro of none/tests/ppc64/isa_3_1_helpers.h
is incomplete because it does not specify the CLOBBERS portion of __asm__.
[See https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html ]
Here is a more-complete statement that tells the compiler
that the __asm__ scribbles on register 21:
__asm__ __volatile__ ("ori 21,21,21"
: /* empty: no outputs from asm to C */
: /* empty: no inputs from C to asm */
: "21" /* clobbers register 21 */
);
Omitting the CLOBBERS is surprising because other __asm__ in the same file
do use it, such as:
__asm__ __volatile__ ("mtcr %0" : : "b"(_arg) : ALLCR );
If the CLOBBERS are specified, the the compiler automatically saves and
restores the clobbered registers, if required because "callee saved" by the
global usage convention. This can be seen by compiling a test such as:
int fn2(int,int,int,int,int,int,int,int,
int,int,int,int,int,int,int,int); /* external */
int fn1(int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)
{
int b1=1, b2=2, b3=3, b4=4, b5=5, b6=6, b7=7, b8=8;
fn2(a1,b1,a2,b2,a3,b3,a4,b4,a5,b5,a6,b6,a7,b7,a8,b8);
__asm__ __volatile__ (
"ori 21,21,21; ori 22,22,22; ori 23,23,23; ori 24,24,24"
: /* empty: no outputs from asm to C */
: /* empty: no inputs from C to asm */
: "21", "22", "23", "24" /* clobbers these registers */
);
fn2(a1,b1,a2,b2,a3,b3,a4,b4,a5,b5,a6,b6,a7,b7,a8,b8);
}
and inspecting the generated code; I used gcc 4.9.2:
fn1:
stwu 1,-160(1)
mflr 0
stw 0,164(1)
stw 21,116(1)
stw 22,120(1)
stw 23,124(1)
stw 24,128(1)
<<snip>>
bl fn2
#APP
# 8 "asm2.c" 1
ori 21,21,21; ori 22,22,22; ori 23,23,23; ori 24,24,24
# 0 "" 2
#NO_APP
<<snip>>
lwz 21,-44(11)
lwz 22,-40(11)
lwz 23,-36(11)
lwz 24,-32(11)
lwz 31,-4(11)
mr 1,11
blr
Not specifying the CLOBBERS may be deliberate, perhaps because the
compiler's automatic save and restore interferes with some other
property of the testing. In that case, there should be a comment
in the code, such as:
/* The __asm__ CLOBBERS are omitted because auto-save and restore
* gets in the way of computing offsets between code blocks.
* Therefore we must save and restore "by hand".
*/
|