Hi Amy, after your change, the compile time for one of our internal test files went from about 1 second to 30 seconds. I have put a repro and the details in PR50628, can you please take a look?

This change appears to be causing 6 or 7 lit tests to hang. Can you take a look and fix or revert?

Hi, we are noticing a regression in the quality of the code generated by the compiler for btver2 after this change.

In D102542#2767735, @phosek wrote:

We're seeing a test failure in prelegalizer-combiner-icmp-to-true-false-known-bits.mir on our 2-stage LTO builder:

FAIL: LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir (2029 of 43749)
******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/llc -debugify-and-strip-all-safe -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="icmp_to_true_false_known_bits" -global-isel -verify-machineinstrs /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir -o - | /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir
--
Exit Code: 2

Command Output (stderr):
--
LLVM ERROR: Invalid rule identifier
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/s/w/ir/x/w/staging/llvm_build/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/x/w/llvm-project/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-icmp-to-true-false-known-bits.mir

--

Would it be possible to revert the change unless the issue can be addressed quickly?

In D98714#2766166, @ABataev wrote:

In D98714#2764805, @dyung wrote:

Hi, this change has caused a regression in the codegen for one of our internal tests. Consider the following code:

__attribute__((noinline))
__m256d add_pd_002(__m256d a, __m256d b) {
  __m256d r = (__m256d){ a[0] + a[1], a[2] + a[3], b[0] + b[1], b[2] + b[3] };
  return __builtin_shufflevector(r, a, 0, -1, 2, 3);
}

If you compile this with "-g0 -O3 -march=btver2", prior to your commit the compiler would generate the following code for the function:

# %bb.0:                                # %entry
        vinsertf128     $1, %xmm1, %ymm0, %ymm0
        vhaddpd %ymm1, %ymm0, %ymm0

But after your change it is now generating the following code:

# %bb.0:                                # %entry
        vextractf128    $1, %ymm1, %xmm2
        vhaddpd %xmm0, %xmm0, %xmm0
        vhaddpd %xmm2, %xmm1, %xmm1
        vperm2f128      $2, %ymm0, %ymm1, %ymm0 # ymm0 = ymm0[0,1],ymm1[0,1]

From your commit description, it sounds like this is expected and will be fixed in a follow-up commit. Is my understanding of this correct?

I see a different result for btver2.

# %bb.0:
        vextractf128    $1, %ymm1, %xmm2
        vhaddpd %xmm1, %xmm0, %xmm0
        vhaddpd %xmm2, %xmm1, %xmm1
        vinsertf128     $1, %xmm1, %ymm0, %ymm0

But I used llvm-11, most probably there is a difference with llvm-12.

Currently, it is impossible to fix this issue. This problem will be fixed after non-power-2 vectorization support in SLP is landed since here we have a build vector of 3 elements (the second index in shuffle is -1 and thus the second sum is optimized out resulting in the build of <4 x double> vector using just 3 insertelement instructions). Looks like previously this patter was recognized by another transformation pass but currently SLP tries to vectorize it

In D98714#2764828, @ABataev wrote:

In D98714#2764805, @dyung wrote:

Hi, this change has caused a regression in the codegen for one of our internal tests. Consider the following code:

__attribute__((noinline))
__m256d add_pd_002(__m256d a, __m256d b) {
  __m256d r = (__m256d){ a[0] + a[1], a[2] + a[3], b[0] + b[1], b[2] + b[3] };
  return __builtin_shufflevector(r, a, 0, -1, 2, 3);
}

If you compile this with "-g0 -O3 -march=btver2", prior to your commit the compiler would generate the following code for the function:

# %bb.0:                                # %entry
        vinsertf128     $1, %xmm1, %ymm0, %ymm0
        vhaddpd %ymm1, %ymm0, %ymm0

But after your change it is now generating the following code:

# %bb.0:                                # %entry
        vextractf128    $1, %ymm1, %xmm2
        vhaddpd %xmm0, %xmm0, %xmm0
        vhaddpd %xmm2, %xmm1, %xmm1
        vperm2f128      $2, %ymm0, %ymm1, %ymm0 # ymm0 = ymm0[0,1],ymm1[0,1]

From your commit description, it sounds like this is expected and will be fixed in a follow-up commit. Is my understanding of this correct?

Hi, could you check if D101555 fixes this issue?

Hi, this change has caused a regression in the codegen for one of our internal tests. Consider the following code:

The test added in this commit was failing on the PS4 bot (https://lab.llvm.org/buildbot/#/builders/139/builds/4059). I've reverted the change in 59b419adc6e608db8d7c31efcc37f34c0b57b7d0 to get the bot green again.

Hi, the file you create here sectdata.txt is being flagged as a virus by Windows Defender in our internal bot causing the file to be automatically deleted and the test to fail. Is there a reason why you need to use that particular string for the file? Can something else be used that won't trigger the antivirus?

This appears to be failing on the PS4 linux bot likely due to the PS4 target has exceptions disabled by default. Can you take a look?

It also fails with the PS4 triple because we default to an external assembler that isn't present: https://lab.llvm.org/buildbot/#/builders/139/builds/2161/steps/6/logs/FAIL__Clang__as-version_s

Hi, our internal x86_64 linux builder is failing after your commit with the error:

FYI, arm64 is aarch64, not arm I believe. I've updated a few tests you added to reflect that.

In D96346#2567215, @jankratochvil wrote:

Unfortunately it fails to build with GCC which LLVM still tries to stay compatible with:
http://lab.llvm.org:8014/#/builders/16/builds/2211
gcc-11.0.0-0.19.fc34.x86_64
gcc-10.2.1-9.fc33.x86_64
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:86:8: error: explicit specialization of 'template<class T> struct llvm::DenseMapInfo' outside its namespace must use a nested-name-specifier [-fpermissive]

86 | struct DenseMapInfo<std::pair<const MCSymbol *, MCSymbolRefExpr::VariantKind>> {
   |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have filed it to an existing GCC bugreport: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92598

It also failed on the PS4 linux bot: http://lab.llvm.org:8014/#/builders/125/builds/125

PS4 changes lgtm.

Updated patch with review feedback.

Thanks, I'll update my patch with your suggestions. I think I was using an older template for the jobs which is why I was using outdated stuff. Sorry about that!

Update removing some unneeded stuff.

lgtm

This issue also affects Windows, so please commit this change.

Hi Pavel, your change caused an assertion failure to be hit in our internal test suite. I have filed a repro with the assertion failure and a callstack in PR48277. Can you please take a look?

The fix would normally be put in a separate change. If it is a small change, you could probably just do a post-commit review. We would like to get this either fixed or reverted so that we can get our internal build bot green again.

Sorry for the lack of context, I made my diffs using a new method this time, but I'll make sure it has the full context next time. Thanks!

LGTM as well. I also tested it and it fails before your change, and passes with your change.

Hi, the test you added seems to pass both before and after your change, is this intended?

I have reverted this change and the follow-up commit in b03c2b8395ba94fb53f1e73a6473faedf628bbd9. Hopefully this should get the bots green again.

In D86360#2278087, @aeubanks wrote:

In D86360#2278086, @wolfgangp wrote:

This commit seems to cause a test failure on some bots, e.g. here.

I think it was already fixed, the bot has been green for the last 2 runs.

I have reverted this change in 8d2d0e84857cb1f2d01456eb433b5172d3a0772b to get the build bots green again.

Looks good, thanks for fixing this for us!

Thanks for the fix!

@craig.topper, I made this fix after your refactor in 3537939cda86f0b5b06233eb99ddc9eb22935008. The main issue I wanted to bring to your attention is that you added FeatureLWP support to ZNVer1 which wasn't there before. If this is correct, please undo my change there. Thanks!

I've reverted this change in g059ba74bb6f6166ca7c1783ef81dd37a5209b758 to get the bots green again.

Matt, this change to the test was causing a failure in the upstream PS4 linux bot here: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68593

Hi, we noticed an issue with the GDB test suite that was bisected back to this change and I have put the details in PR46052. Can you take a look?