David Greene via llvm-dev
2018-Dec-21 15:30 UTC
[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 at 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
David Greene via llvm-dev
2018-Dec-21 15:43 UTC
[llvm-dev] [OpenMP][AArch64][GlobalISel] AArch64 OMPT tests failing
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 at 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
Jonas Hahnfeld via llvm-dev
2018-Dec-21 16:16 UTC
[llvm-dev] [OpenMP][AArch64][GlobalISel] AArch64 OMPT tests failing
On 2018-12-21 16:43, David Greene wrote:> 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>Could you post a bit more of context? It would be helpful to see what's the address of the generated label. This should be somewhere before the call to 'printf'.> This code is the same with and without GlobalISel.Hmm, does this mean it's also failing without GlobalISel? If not then there must be something different, it might just be somewhere else... In the above code, the last line is an unconditional branch to the next instruction. If I remember correctly, that was the issue we identified back then. Could you post the asm without GlobalISel for comparison?> 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?The test should already deal with this: https://github.com/llvm-mirror/openmp/blob/master/runtime/test/ompt/callback.h#L160. The NOP is always there, the store is related to functions that return a value (like omp_control_tool does). Thanks, Jonas> > -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 at 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