[llvm] ee75794 - [ARM][TypePromotion] Enable by default

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 21 16:18:03 PST 2019


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/20191221/bbe5da46/attachment.html>


More information about the llvm-commits mailing list