search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.31k stars 11.69k forks source link

[ARM] undesired debug instructions causes assertion fails in ConvertVPTBlocks #50159

Closed llvmbot closed 3 years ago

llvmbot commented 3 years ago
Bugzilla Link 50815
Resolution FIXED
Resolved on Jun 29, 2021 13:13
Version 12.0
OS Linux
Blocks llvm/llvm-project#48661
Attachments failed case, bugpoint found another assertion fails, though not related to this
Reporter LLVM Bugzilla Contributor
CC @DMG862,@efriedma-quic,@smithp35,@tstellar
Fixed by commit(s) e0c605f6383c5a3aa8f5fa34ed4be9dc51b4a2ae f22b4c7122bc86c640f75fd8b2b165d124204105

Extended Description

Found when trying to compile newlib. LLVM is on the branch official/release/12.x I tried the commit in the main branch: 9872551ca09b David Green o [ARM] Skip debug during vpt block creation but its not helpful.

I think ConvertVPTBlocks should also skip debug instructions at around line 1527.

4c958a03-4ad7-407c-a85b-320d9a7221b0 commented 3 years ago

These made their way onto the llvm 12 branch? Thanks for doing that.

I'll close this ticket. Let us know if there are any other problems.

4c958a03-4ad7-407c-a85b-320d9a7221b0 commented 3 years ago

I had another report today from someone using llvm-12 for MVE work and hitting an assert. There is an extra patch on that branch that involves the cherry-pick of 465df35355ec30ab8c60ef1c0c156a6b22bda7d4 (with an adjustment for the differences since that pass has been largely rewritten since the branch, and we don't want to take all those changes over).

llvmbot commented 3 years ago

OK. I can use this github fork first. Thanks guys.

4c958a03-4ad7-407c-a85b-320d9a7221b0 commented 3 years ago

It turns out that the llvm 12 branch point was quite a while ago. This is the collection of patches needed to fix the crashing issues that have come up since then for MVE:

9872551ca09b60f24d9090e7681de6fc9627ce33 [ARM] Skip debug during vpt block creation f22b4c7122bc86c640f75fd8b2b165d124204105 [ARM] Handle debug instrs in ARM Low Overhead Loop pass 2176be556b448361a35c01cfedd5d3fd54b3e2b9 [ARM] Guard against loop variant gather ptr operands 258e2e9a0bdc1e0b0ade99a27de4190010cbee78 [ARM] Ensure loop invariant active.lane.mask operands e0c605f6383c5a3aa8f5fa34ed4be9dc51b4a2ae [ARM] Ensure instructions are simplified prior to GatherScatter lowering.

These (and 2 extra test-only cherry picks) are now up at https://github.com/DMG862/llvm-project/commits/release12MVE, which pass the ninja check-llvm tests. (And don't crash on any of the new mve tests from main).

Tim doesn't really have any part in this area of the code (but an extra set of eyes is always good). These patches fix crashes in MVE, but only effect MVE passes so should be relatively safe.

tstellar commented 3 years ago

The fix does not apply cleanly, could someone backport this and push a branch to their local github fork?

tstellar commented 3 years ago

Hi Tim,

What is your opinion on backporting this?

https://reviews.llvm.org/rGe0c605f6383c5a3aa8f5fa34ed4be9dc51b4a2ae https://reviews.llvm.org/rGf22b4c7122bc86c640f75fd8b2b165d124204105

tstellar commented 3 years ago

This request was submitted after the deadline, so we may not be able to get this merged in time.

llvmbot commented 3 years ago

Hi Dave Green, Thanks for your help. They work like a charm. Would they be merged into release/12.x?

4c958a03-4ad7-407c-a85b-320d9a7221b0 commented 3 years ago

Hello

The loop with no instructions may have been fixed by https://reviews.llvm.org/rGe0c605f6383c5a3aa8f5fa34ed4be9dc51b4a2ae. It is fixed on trunk and seems to be a separate issue, not related to debug.

There was https://reviews.llvm.org/rGf22b4c7122bc86c640f75fd8b2b165d124204105 for the ARMLowOverheadLoops pass, also dealing with debug instructions. Those together allow the original test case to pass.

llvmbot commented 3 years ago

assigned to @TNorthover