[llvm] ee75794 - [ARM][TypePromotion] Enable by default
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 22 11:27:53 PST 2019
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> 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> 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
>> 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/20191222/129eea00/attachment.html>
More information about the llvm-commits
mailing list