[llvm-dev] [OpenMP][AArch64][GlobalISel] AArch64 OMPT tests failing

[David Greene via llvm-dev]

I took a look at the asm for control_tool.c.  I'm not familiar with OMPT so I
might have to get our OMP guys involved, but I think this is the relevant piece
of code:

  403648:       97fff472        bl      400810 <omp_control_tool at plt>
  40364c:       d503201f        nop
  403650:       b9000fe0        str     w0, [sp,#12]
  403654:       14000001        b       403658 <.omp_outlined.+0xa8>

This code is the same with and without GlobalISel.

Bug 36313 mentions that if the called routine returns a value, the return
address will be off due to an added store.  omp_control_tool does indeed
return a value.  Is this simply a matter of  fixing the test?

                                 -David

________________________________________
From: Openmp-dev <openmp-dev-bounces at lists.llvm.org> on behalf of David Greene via Openmp-dev <openmp-dev at lists.llvm.org>
Sent: Friday, December 21, 2018 9:30:19 AM
To: Jonas Hahnfeld
Cc: llvm-dev at lists.llvm.org; aemerson at apple.com; Petr Pavlu; openmp-dev at lists.llvm.org
Subject: Re: [Openmp-dev] [llvm-dev] [OpenMP][AArch64][GlobalISel] AArch64 OMPT tests failing

Curious.  I removed -fno-experimental-isel and all of the tests *except*
control_tool.c passed.  I would have expected all of them to pass if
blockaddress works.

I'll try to look at some asm and see what's going on.

                         -David

Jonas Hahnfeld <hahnjo at hahnjo.de> writes:

> Hi David,
>
> I was the one who originally added the flag to fix failures related to
> GlobalISel. This was because first versions of GlobalISel didn't know
> how to select 'blockaddress', but this should have been fixed (see
> https://bugs.llvm.org/show_bug.cgi?id=36390) and available in both 7.0
> and trunk.
> There has been some discussion prior to that in
> https://bugs.llvm.org/show_bug.cgi?id=36313, maybe you could look into
> what's still wrong on AArch64?
>
> Cheers,
> Jonas
>
> On 2018-12-20 22:34, David Greene via llvm-dev wrote:
>> We're seeing OMPT tests fail on AArch64:
>>
>>     libomp :: ompt/misc/control_tool.c
>>     libomp :: ompt/synchronization/master.c
>>     libomp :: ompt/synchronization/taskwait.c
>>
>> The failure mode is similar for all of them:
>>
>> openmp/runtime/test/ompt/misc/control_tool.c:26:17: error: CHECK-NEXT:
>> expected string not found in input
>>  // CHECK-NEXT: {{^}}[[MASTER_ID]]:
>> current_address={{.*}}[[RETURN_ADDRESS]]
>>                 ^
>> <stdin>:9:1: note: scanning from here
>> 281474976710657: current_address=0x402cf4 or 0x402cf0
>> ^
>> <stdin>:9:1: note: with variable "MASTER_ID" equal to "281474976710657"
>> 281474976710657: current_address=0x402cf4 or 0x402cf0
>> ^
>> <stdin>:9:1: note: with variable "RETURN_ADDRESS" equal to "0x402cec"
>> 281474976710657: current_address=0x402cf4 or 0x402cf0
>> ^
>> <stdin>:9:13: note: possible intended match here
>> 281474976710657: current_address=0x402cf4 or 0x402cf0
>>             ^
>>
>> I bisected the control_tool.c failure to:
>>
>> 3834f852008a82e361d325ec7b1c3fee0dc783c3 is the first bad commit
>> commit 3834f852008a82e361d325ec7b1c3fee0dc783c3
>> Author: Petr Pavlu <petr.pavlu at arm.com>
>> Date:   Thu Nov 29 12:56:32 2018 +0000
>>
>>     [GlobalISel] Make EnableGlobalISel always set when GISel is enabled
>>
>>     Change meaning of TargetOptions::EnableGlobalISel. The flag was
>>     previously set only when a target switched on GlobalISel but it
>> is now
>>     always set when the GlobalISel pipeline is enabled. This makes
>> the flag
>>     consistent with TargetOptions::EnableFastISel and allows its use in
>>     other parts of the compiler to determine when GlobalISel is
>> enabled.
>>
>>     The EnableGlobalISel flag had previouly only one use in
>>     TargetPassConfig::isGlobalISelAbortEnabled(). The method used
>> its value
>>     to determine if GlobalISel was enabled by a target and returned
>> false in
>>     such a case. To preserve the current behaviour, a new flag
>>     TargetOptions::GlobalISelAbort is introduced to separately
>> record the
>>     abort behaviour.
>>
>>     Differential Revision: https://reviews.llvm.org/D54518>
>>
>>     git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@347861> 91177308-0d34-0410-b5e6-96231b3b80d8
>>
>> Is it possible this commit changed the behavior of clang's
>> -fno-experimental-isel?  The OpenMP tests use that flag.
>>
>>                             -David
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
Openmp-dev mailing list
Openmp-dev at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev