[llvm] ee75794 - [ARM][TypePromotion] Enable by default
Sam Parker via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 23 01:56:46 PST 2019
Many thanks, that makes sense. I've uploaded a patch to address the issue: https://reviews.llvm.org/D71832
Sam Parker
Compilation Tools Engineer | Arm
. . . . . . . . . . . . . . . . . . . . . . . . . . .
Arm.com
________________________________
From: Reid Kleckner <rnk at google.com>
Sent: 22 December 2019 19:27
To: Sam Parker <Sam.Parker at arm.com>; Sam Parker <llvmlistbot at llvm.org>; Hans Wennborg <hwennborg at google.com>; Nico Weber <thakis at chromium.org>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] ee75794 - [ARM][TypePromotion] Enable by default
Reverted in b2c1ba5b1f8049cdacec1a111c2ef267cd5acff5.
TypePromotion is crashing on this instruction:
%26 = trunc i256 %25 to i256, !dbg !77
This code tries to get the max i256 value as a 64-bit integer, and it crashes:
https://github.com/llvm/llvm-project/blob/a4aa40cebc9b4a1d8a7dafddd65d99c4fd53ebec/llvm/lib/CodeGen/TypePromotion.cpp#L671
auto *Trunc = cast<TruncInst>(V); // See i256 trunc above
Builder.SetInsertPoint(Trunc);
IntegerType *SrcTy = cast<IntegerType>(Trunc->getOperand(0)->getType());
IntegerType *DestTy = cast<IntegerType>(TruncTysMap[Trunc][0]);
unsigned NumBits = DestTy->getScalarSizeInBits(); // appears to be 256
ConstantInt *Mask =
ConstantInt::get(SrcTy, APInt::getMaxValue(NumBits).getZExtValue()); // getZExtValue asserts
Value *Masked = Builder.CreateAnd(Trunc->getOperand(0), Mask);
llvm::TypePromotion::TypeSize appears to be 80 at this point. I have some gdb logs including the function IR dump linked below. According to Hans, compiling this file in isolation will not repro the issue, you have to do it in the context of ThinLTO, which makes me suspect a race.
https://reviews.llvm.org/P8177
On Sat, Dec 21, 2019 at 4:18 PM Reid Kleckner <rnk at google.com<mailto:rnk at google.com>> wrote:
This has been causing problems for our ThinLTO builds of Chromium:
https://bugs.chromium.org/p/chromium/issues/detail?id=1033863#c20
I have a gdb session open where this getZExtValue call asserts (i.e. NumBits is >= 65):
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/TypePromotion.cpp#L671
I haven't debugged why this is yet, and it will be slow going over the holidays. I think we should revert this in the meantime.
I also see what looks like a shallow race condition bug, but I haven't studied the code well enough to confirm it:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/TypePromotion.cpp#L157
The global variable TypePromotion::TypeSize is modified every time the pass is run, which is obviously thread hostile. ThinLTO uses multiple threads to do codegen, and that might be the root of the problem. In the meantime, I will revert this flag flip.
On Wed, Dec 11, 2019 at 2:00 AM Sam Parker via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: Sam Parker
Date: 2019-12-11T10:00:16Z
New Revision: ee7579409b7d940c4e1314d126e900db30c4edff
URL: https://github.com/llvm/llvm-project/commit/ee7579409b7d940c4e1314d126e900db30c4edff
DIFF: https://github.com/llvm/llvm-project/commit/ee7579409b7d940c4e1314d126e900db30c4edff.diff
LOG: [ARM][TypePromotion] Enable by default
Enable the TypePromotion pass my default (again).
This patch was originally committed in 393dacacf7e7.
This patch was reverted in a38396939c54.
Differential Revision: https://reviews.llvm.org/D70998
Added:
Modified:
llvm/lib/CodeGen/TypePromotion.cpp
llvm/test/Transforms/TypePromotion/ARM/calls.ll
llvm/test/Transforms/TypePromotion/ARM/casts.ll
llvm/test/Transforms/TypePromotion/ARM/clear-structures.ll
llvm/test/Transforms/TypePromotion/ARM/icmps.ll
llvm/test/Transforms/TypePromotion/ARM/phis-ret.ll
llvm/test/Transforms/TypePromotion/ARM/pointers.ll
llvm/test/Transforms/TypePromotion/ARM/signed-icmps.ll
llvm/test/Transforms/TypePromotion/ARM/signed.ll
llvm/test/Transforms/TypePromotion/ARM/switch.ll
llvm/test/Transforms/TypePromotion/ARM/wrapping.ll
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/TypePromotion.cpp b/llvm/lib/CodeGen/TypePromotion.cpp
index d78589c4a056..000cd366dd69 100644
--- a/llvm/lib/CodeGen/TypePromotion.cpp
+++ b/llvm/lib/CodeGen/TypePromotion.cpp
@@ -46,7 +46,7 @@
using namespace llvm;
static cl::opt<bool>
-DisablePromotion("disable-type-promotion", cl::Hidden, cl::init(true),
+DisablePromotion("disable-type-promotion", cl::Hidden, cl::init(false),
cl::desc("Disable type promotion pass"));
// The goal of this pass is to enable more efficient code generation for
@@ -902,16 +902,34 @@ bool TypePromotion::TryToPromote(Value *V, unsigned PromotedWidth) {
for (auto *I : CurrentVisited)
I->dump();
);
+
+ // Check that promoting this at the IR level is most likely beneficial. It's
+ // more likely if we're operating over multiple blocks and handling wrapping
+ // instructions.
unsigned ToPromote = 0;
+ unsigned NonFreeArgs = 0;
+ SmallPtrSet<BasicBlock*, 4> Blocks;
for (auto *V : CurrentVisited) {
- if (Sources.count(V))
+ if (auto *I = dyn_cast<Instruction>(V))
+ Blocks.insert(I->getParent());
+
+ if (Sources.count(V)) {
+ if (auto *Arg = dyn_cast<Argument>(V)) {
+ if (!Arg->hasZExtAttr() && !Arg->hasSExtAttr())
+ ++NonFreeArgs;
+ }
continue;
+ }
+
if (Sinks.count(cast<Instruction>(V)))
continue;
+
++ToPromote;
}
- if (ToPromote < 2)
+ // DAG optimisations should be able to handle these cases better, especially
+ // for function arguments.
+ if (ToPromote < 2 || (Blocks.size() == 1 && (NonFreeArgs > SafeWrap.size())))
return false;
Promoter->Mutate(OrigTy, PromotedWidth, CurrentVisited, Sources, Sinks,
diff --git a/llvm/test/Transforms/TypePromotion/ARM/calls.ll b/llvm/test/Transforms/TypePromotion/ARM/calls.ll
index cd273c06150f..8c14a3a076f1 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/calls.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/calls.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define i8 @call_with_imms(i8* %arg) {
; CHECK-LABEL: @call_with_imms(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/casts.ll b/llvm/test/Transforms/TypePromotion/ARM/casts.ll
index 70fa617115e8..66c713ce3058 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/casts.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/casts.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define i16 @dsp_trunc(i32 %arg0, i32 %arg1, i16* %gep0, i16* %gep1) {
; CHECK-LABEL: @dsp_trunc(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/clear-structures.ll b/llvm/test/Transforms/TypePromotion/ARM/clear-structures.ll
index 117c4c0d5c8a..aae729fcc92c 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/clear-structures.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/clear-structures.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define i32 @clear_structures(i8* nocapture readonly %fmt, [1 x i32] %ap.coerce, i8* %out, void (i32, i8*)* nocapture %write) {
; CHECK-LABEL: @clear_structures(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/icmps.ll b/llvm/test/Transforms/TypePromotion/ARM/icmps.ll
index 6dda15c309b4..8d33c345c640 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/icmps.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/icmps.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define i32 @test_ult_254_inc_imm(i8 zeroext %x) {
; CHECK-LABEL: @test_ult_254_inc_imm(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/phis-ret.ll b/llvm/test/Transforms/TypePromotion/ARM/phis-ret.ll
index e79e4ff1bdb2..12001a40aa08 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/phis-ret.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/phis-ret.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
; Check that the arguments are extended but then nothing else is.
; This also ensures that the pass can handle loops.
diff --git a/llvm/test/Transforms/TypePromotion/ARM/pointers.ll b/llvm/test/Transforms/TypePromotion/ARM/pointers.ll
index 3c5f097b1b92..7369b18bbd64 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/pointers.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/pointers.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define void @phi_pointers(i16* %a, i16* %b, i8 zeroext %M, i8 zeroext %N) {
; CHECK-LABEL: @phi_pointers(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/signed-icmps.ll b/llvm/test/Transforms/TypePromotion/ARM/signed-icmps.ll
index dfdd4c10ae87..3655ff0e7e63 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/signed-icmps.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/signed-icmps.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define i8 @eq_sgt(i8* %x, i8 *%y, i8 zeroext %z) {
; CHECK-LABEL: @eq_sgt(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/signed.ll b/llvm/test/Transforms/TypePromotion/ARM/signed.ll
index 143220a53b5c..e2fabb93fbd5 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/signed.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/signed.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
; Test to check that ARMCodeGenPrepare doesn't optimised away sign extends.
define i16 @test_signed_load(i16* %ptr) {
diff --git a/llvm/test/Transforms/TypePromotion/ARM/switch.ll b/llvm/test/Transforms/TypePromotion/ARM/switch.ll
index 6736ebeea4c4..058101f34951 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/switch.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/switch.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define void @truncate_source_phi_switch(i8* %memblock, i8* %store, i16 %arg) {
; CHECK-LABEL: @truncate_source_phi_switch(
diff --git a/llvm/test/Transforms/TypePromotion/ARM/wrapping.ll b/llvm/test/Transforms/TypePromotion/ARM/wrapping.ll
index 23e50dec0ca1..346114aac3cd 100644
--- a/llvm/test/Transforms/TypePromotion/ARM/wrapping.ll
+++ b/llvm/test/Transforms/TypePromotion/ARM/wrapping.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -mtriple=arm -type-promotion -verify -disable-type-promotion=false -S %s -o - | FileCheck %s
+; RUN: opt -mtriple=arm -type-promotion -verify -S %s -o - | FileCheck %s
define zeroext i16 @overflow_add(i16 zeroext %a, i16 zeroext %b) {
; CHECK-LABEL: @overflow_add(
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191223/bf79ff9a/attachment.html>
More information about the llvm-commits
mailing list