[llvm] 1a02aae - [SCCP] Use SimplifyBinOp for non-integer constant/expressions & overdef.
Benjamin Kramer via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 13 02:27:34 PDT 2020
I see this making llvm crash with an assertion failure. Reverted in
06408451bf12d4baed1fb1312d8af6e6bbb6a797, test case in commit message.
On Fri, Apr 10, 2020 at 12:05 PM Florian Hahn via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Florian Hahn
> Date: 2020-04-10T11:02:57+01:00
> New Revision: 1a02aaeaa4f8675490da38ee8cb0d4a6d39815dd
>
> URL: https://github.com/llvm/llvm-project/commit/1a02aaeaa4f8675490da38ee8cb0d4a6d39815dd
> DIFF: https://github.com/llvm/llvm-project/commit/1a02aaeaa4f8675490da38ee8cb0d4a6d39815dd.diff
>
> LOG: [SCCP] Use SimplifyBinOp for non-integer constant/expressions & overdef.
>
> For non-integer constants/expressions and overdefined, I think we can
> just use SimplifyBinOp to do common folds. By just passing a context
> with the DL, SimplifyBinOp should not try to get additional information
> from looking at definitions.
>
> For overdefined values, it should be enough to just pass the original
> operand.
>
> Note: The comment before the `if (isconstant(V1State)...` was wrong
> originally: isConstant() also matches integer ranges with a single
> element. It is correct now.
>
> Reviewers: efriedma, davide, mssimpso, aartbik
>
> Reviewed By: efriedma
>
> Differential Revision: https://reviews.llvm.org/D76459
>
> Added:
>
>
> Modified:
> llvm/lib/Transforms/Scalar/SCCP.cpp
> llvm/test/Transforms/SCCP/ub-shift.ll
> llvm/test/Transforms/SCCP/vector-bitcast.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
> index c8d7c4de37fc..2953c30fe49a 100644
> --- a/llvm/lib/Transforms/Scalar/SCCP.cpp
> +++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
> @@ -28,6 +28,7 @@
> #include "llvm/ADT/Statistic.h"
> #include "llvm/Analysis/ConstantFolding.h"
> #include "llvm/Analysis/GlobalsModRef.h"
> +#include "llvm/Analysis/InstructionSimplify.h"
> #include "llvm/Analysis/TargetLibraryInfo.h"
> #include "llvm/Analysis/ValueLattice.h"
> #include "llvm/Analysis/ValueLatticeUtils.h"
> @@ -369,8 +370,10 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
> // markConstant - Make a value be marked as "constant". If the value
> // is not already a constant, add it to the instruction work list so that
> // the users of the instruction are updated later.
> - bool markConstant(ValueLatticeElement &IV, Value *V, Constant *C) {
> - if (!IV.markConstant(C)) return false;
> + bool markConstant(ValueLatticeElement &IV, Value *V, Constant *C,
> + bool MayIncludeUndef = false) {
> + if (!IV.markConstant(C, MayIncludeUndef))
> + return false;
> LLVM_DEBUG(dbgs() << "markConstant: " << *C << ": " << *V << '\n');
> pushToWorkList(IV, V);
> return true;
> @@ -954,23 +957,28 @@ void SCCPSolver::visitBinaryOperator(Instruction &I) {
> if (V1State.isOverdefined() && V2State.isOverdefined())
> return (void)markOverdefined(&I);
>
> - // Both operands are non-integer constants or constant expressions.
> + // If either of the operands is a constant, try to fold it to a constant.
> // TODO: Use information from notconstant better.
> - if (isConstant(V1State) && isConstant(V2State)) {
> - Constant *C = ConstantExpr::get(I.getOpcode(), getConstant(V1State),
> - getConstant(V2State));
> - // X op Y -> undef.
> - if (isa<UndefValue>(C))
> - return;
> - return (void)markConstant(IV, &I, C);
> + if ((V1State.isConstant() || V2State.isConstant())) {
> + Value *V1 = isConstant(V1State) ? getConstant(V1State) : I.getOperand(0);
> + Value *V2 = isConstant(V2State) ? getConstant(V2State) : I.getOperand(1);
> + Value *R = SimplifyBinOp(I.getOpcode(), V1, V2, SimplifyQuery(DL));
> + auto *C = dyn_cast_or_null<Constant>(R);
> + if (C) {
> + // X op Y -> undef.
> + if (isa<UndefValue>(C))
> + return;
> + // Conservatively assume that the result may be based on operands that may
> + // be undef.
> + return (void)markConstant(IV, &I, C, /*MayIncludeUndef=*/true);
> + }
> }
>
> // Only use ranges for binary operators on integers.
> if (!I.getType()->isIntegerTy())
> return markOverdefined(&I);
>
> - // Operands are either constant ranges, notconstant, overdefined or one of the
> - // operands is a constant.
> + // Try to simplify to a constant range.
> ConstantRange A = ConstantRange::getFull(I.getType()->getScalarSizeInBits());
> ConstantRange B = ConstantRange::getFull(I.getType()->getScalarSizeInBits());
> if (V1State.isConstantRange())
>
> diff --git a/llvm/test/Transforms/SCCP/ub-shift.ll b/llvm/test/Transforms/SCCP/ub-shift.ll
> index 6e15d6b2bccd..fbcaef422870 100644
> --- a/llvm/test/Transforms/SCCP/ub-shift.ll
> +++ b/llvm/test/Transforms/SCCP/ub-shift.ll
> @@ -3,10 +3,8 @@
>
> define void @shift_undef_64(i64* %p) {
> ; CHECK-LABEL: @shift_undef_64(
> -; CHECK-NEXT: [[R1:%.*]] = lshr i64 -1, 4294967296
> -; CHECK-NEXT: store i64 [[R1]], i64* [[P:%.*]]
> -; CHECK-NEXT: [[R2:%.*]] = ashr i64 -1, 4294967297
> -; CHECK-NEXT: store i64 [[R2]], i64* [[P]]
> +; CHECK-NEXT: store i64 0, i64* [[P:%.*]]
> +; CHECK-NEXT: store i64 -1, i64* [[P]]
> ; CHECK-NEXT: [[R3:%.*]] = shl i64 -1, 4294967298
> ; CHECK-NEXT: store i64 [[R3]], i64* [[P]]
> ; CHECK-NEXT: ret void
> @@ -25,10 +23,8 @@ define void @shift_undef_64(i64* %p) {
>
> define void @shift_undef_65(i65* %p) {
> ; CHECK-LABEL: @shift_undef_65(
> -; CHECK-NEXT: [[R1:%.*]] = lshr i65 2, -18446744073709551615
> -; CHECK-NEXT: store i65 [[R1]], i65* [[P:%.*]]
> -; CHECK-NEXT: [[R2:%.*]] = ashr i65 4, -18446744073709551615
> -; CHECK-NEXT: store i65 [[R2]], i65* [[P]]
> +; CHECK-NEXT: store i65 0, i65* [[P:%.*]]
> +; CHECK-NEXT: store i65 0, i65* [[P]]
> ; CHECK-NEXT: [[R3:%.*]] = shl i65 1, -18446744073709551615
> ; CHECK-NEXT: store i65 [[R3]], i65* [[P]]
> ; CHECK-NEXT: ret void
> @@ -47,10 +43,8 @@ define void @shift_undef_65(i65* %p) {
>
> define void @shift_undef_256(i256* %p) {
> ; CHECK-LABEL: @shift_undef_256(
> -; CHECK-NEXT: [[R1:%.*]] = lshr i256 2, 18446744073709551617
> -; CHECK-NEXT: store i256 [[R1]], i256* [[P:%.*]]
> -; CHECK-NEXT: [[R2:%.*]] = ashr i256 4, 18446744073709551618
> -; CHECK-NEXT: store i256 [[R2]], i256* [[P]]
> +; CHECK-NEXT: store i256 0, i256* [[P:%.*]]
> +; CHECK-NEXT: store i256 0, i256* [[P]]
> ; CHECK-NEXT: [[R3:%.*]] = shl i256 1, 18446744073709551619
> ; CHECK-NEXT: store i256 [[R3]], i256* [[P]]
> ; CHECK-NEXT: ret void
> @@ -69,10 +63,8 @@ define void @shift_undef_256(i256* %p) {
>
> define void @shift_undef_511(i511* %p) {
> ; CHECK-LABEL: @shift_undef_511(
> -; CHECK-NEXT: [[R1:%.*]] = lshr i511 -1, 1208925819614629174706276
> -; CHECK-NEXT: store i511 [[R1]], i511* [[P:%.*]]
> -; CHECK-NEXT: [[R2:%.*]] = ashr i511 -2, 1208925819614629174706200
> -; CHECK-NEXT: store i511 [[R2]], i511* [[P]]
> +; CHECK-NEXT: store i511 0, i511* [[P:%.*]]
> +; CHECK-NEXT: store i511 -1, i511* [[P]]
> ; CHECK-NEXT: [[R3:%.*]] = shl i511 -3, 1208925819614629174706180
> ; CHECK-NEXT: store i511 [[R3]], i511* [[P]]
> ; CHECK-NEXT: ret void
>
> diff --git a/llvm/test/Transforms/SCCP/vector-bitcast.ll b/llvm/test/Transforms/SCCP/vector-bitcast.ll
> index 35312034c65b..b032085083c6 100644
> --- a/llvm/test/Transforms/SCCP/vector-bitcast.ll
> +++ b/llvm/test/Transforms/SCCP/vector-bitcast.ll
> @@ -2,8 +2,7 @@
>
> target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128-n8:16:32-S128"
>
> -; FIXME: Add back support for handling special values of vector/fp types.
> -; CHECK: store volatile <2 x i64> %and.i119.i, <2 x i64>* %p
> +; CHECK: store volatile <2 x i64> zeroinitializer, <2 x i64>* %p
> ; rdar://11324230
>
> define void @foo(<2 x i64>* %p) nounwind {
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list