[instcombine] remove fsub to fneg hacks; only emit fneg

Summary: Rewrite the fsub-0.0 idiom to fneg and always emit fneg for fp
negation. This also extends the scalarization cost in instcombine for

[instcombine] remove fsub to fneg hacks; only emit fneg

Summary: Rewrite the fsub-0.0 idiom to fneg and always emit fneg for fp
negation. This also extends the scalarization cost in instcombine for unary
operators to result in the same IR rewrites for fneg as for the idiom.

Reviewed By: cameron.mcinally

Differential Revision: https://reviews.llvm.org/D75467

show more ...

[InstCombine] Use IRBuilder to create bitcast

This makes sure that the constant expression bitcast goes through
target-dependent constant folding, and thus avoids an additional
iteration of InstComb

[InstCombine] Use IRBuilder to create bitcast

This makes sure that the constant expression bitcast goes through
target-dependent constant folding, and thus avoids an additional
iteration of InstCombine.

show more ...

[ConstantFolding] Always return something from ConstantFoldConstant

Spin-off from D75407. As described there, ConstantFoldConstant()
currently returns null for non-ConstantExpr/ConstantVector inputs

[ConstantFolding] Always return something from ConstantFoldConstant

Spin-off from D75407. As described there, ConstantFoldConstant()
currently returns null for non-ConstantExpr/ConstantVector inputs,
but otherwise always returns non-null, independently of whether
any folding has happened or not.

This is confusing and makes consumer code more complicated.
I would expect either that ConstantFoldConstant() returns only if
it actually folded something, or that it always returns non-null.
I'm going to the latter possibility here, which appears to be more
useful considering existing usage.

Differential Revision: https://reviews.llvm.org/D75543

show more ...

Remove BinaryOperator::CreateFNeg

Use UnaryOperator::CreateFNeg instead.

Summary:
With the introduction of the native fneg instruction, the
fsub -0.0, %x idiom is obsolete. This patch makes LLVM
em

Remove BinaryOperator::CreateFNeg

Use UnaryOperator::CreateFNeg instead.

Summary:
With the introduction of the native fneg instruction, the
fsub -0.0, %x idiom is obsolete. This patch makes LLVM
emit fneg instead of the idiom in all places.

Reviewed By: cameron.mcinally

Differential Revision: https://reviews.llvm.org/D75130

show more ...

[IRBuilder] Prefer InsertPointGuard over full copy; NFC

Don't copy the IRBuilder when an InsertPointGuard would also do.

[InstCombine] fix operands of shouldChangeType() for casted phi transform

This is a bug noted in the recent D72733 and seen
in the similar transform just above the changed source code.

I added test

[InstCombine] fix operands of shouldChangeType() for casted phi transform

This is a bug noted in the recent D72733 and seen
in the similar transform just above the changed source code.

I added tests with illegal types and zexts to show the bug -
we could transform legal phi ops to illegal, etc. I did not add
tests with trunc because we won't see any diffs on those patterns.
That is because InstCombiner::SliceUpIllegalIntegerPHI() appears to
do those transforms independently of datalayout. It can also create
more casts than are present in existing code.

There are some existing regression tests that do not include a
datalayout that would be altered by this fix. I assumed that the
lack of a datalayout in those regression files is an oversight, so
I added the minimal layout (make i32 legal) necessary to preserve
behavior on those tests.

Differential Revision: https://reviews.llvm.org/D73907

show more ...

[InstCombine] Add replaceOperand() helper

Adds a replaceOperand() helper, which is like Instruction.setOperand()
but adds the old operand to the worklist. This reduces the amount of
missing or incor

[InstCombine] Add replaceOperand() helper

Adds a replaceOperand() helper, which is like Instruction.setOperand()
but adds the old operand to the worklist. This reduces the amount of
missing or incorrect worklist management.

This only applies the helper to a relatively small subset of
setOperand() calls in InstCombine, namely those of the pattern
`I.setOperand(); return &I;`, where it is most obviously applicable.

Differential Revision: https://reviews.llvm.org/D73803

show more ...

[InstCombine] Rename worklist methods; NFC

This renames Worklist.AddDeferred() to Worklist.add() and
Worklist.Add() to Worklist.push(). The intention here is that
Worklist.add() should be the go-to

[InstCombine] Rename worklist methods; NFC

This renames Worklist.AddDeferred() to Worklist.add() and
Worklist.Add() to Worklist.push(). The intention here is that
Worklist.add() should be the go-to method for explicit worklist
management, while the raw Worklist.push() is mostly for
InstCombine internals. I will then migrate uses of Worklist.push()
to Worklist.add() in followup changes.

As suggested by spatel on D73411 I'm also changing the remaining
method names to lowercase first character, in line with current
coding standards.

Differential Revision: https://reviews.llvm.org/D73745

show more ...

[InstCombine] allow more narrowing of casted select

D47163 created a rule that we should not change the casted
type of a select when we have matching types in its compare condition.
That was intende

[InstCombine] allow more narrowing of casted select

D47163 created a rule that we should not change the casted
type of a select when we have matching types in its compare condition.
That was intended to help vector codegen, but it also could create
situations where we miss subsequent folds as shown in PR44545:
https://bugs.llvm.org/show_bug.cgi?id=44545

By using shouldChangeType(), we can continue to get the vector folds
(because we always return false for vector types). But we also solve
the motivating bug because it's ok to narrow the scalar select in that
example.

Our canonicalization rules around select are a mess, but AFAICT, this
will not induce any infinite looping from the reverse transform (but
we'll need to watch for that possibility if committed).

Side note: there's a similar use of shouldChangeType() for phi ops
just below this diff, and the source and destination types appear to
be reversed.

Differential Revision: https://reviews.llvm.org/D72733

show more ...

[InstCombine] Fix infinite loop due to bitcast <-> phi transforms

Fix for https://bugs.llvm.org/show_bug.cgi?id=44245.

The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up
fighting against

[InstCombine] Fix infinite loop due to bitcast <-> phi transforms

Fix for https://bugs.llvm.org/show_bug.cgi?id=44245.

The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up
fighting against each other, because optimizeBitCastFromPhi()
assumes that bitcasts of loads will get folded. This doesn't
happen here, because a dangling phi node prevents the one-use
fold in https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp#L620-L628 from triggering.

This patch fixes the issue by explicitly performing the load
combine as part of the bitcast of phi transform. Other attempts
to force the load to be combined first were ultimately too
unreliable.

Differential Revision: https://reviews.llvm.org/D71164

show more ...

[InstCombine] Fix user iterator invalidation in bitcast of phi transform

This fixes the issue encountered in D71164. Instead of using a
range-based for, manually iterate over the users and advance t

[InstCombine] Fix user iterator invalidation in bitcast of phi transform

This fixes the issue encountered in D71164. Instead of using a
range-based for, manually iterate over the users and advance the
iterator beforehand, so we do not skip any users due to iterator
invalidation.

Differential Revision: https://reviews.llvm.org/D72657

show more ...

Revert "[InstCombine] fold zext of masked bit set/clear"

This reverts commit a041c4ec6f7aa659b235cb67e9231a05e0a33b7d.

This looks like a non-trivial change and there has been no code
reviews (at le

Revert "[InstCombine] fold zext of masked bit set/clear"

This reverts commit a041c4ec6f7aa659b235cb67e9231a05e0a33b7d.

This looks like a non-trivial change and there has been no code
reviews (at least there were no phabricator revisions attached to the
commit description). It is also causing a regression in one of our
downstream integration tests, we haven't been able to come up with a
minimal reproducer yet.

show more ...

[InstCombine] fold zext of masked bit set/clear

This does not solve PR17101, but it is one of the
underlying diffs noted here:
https://bugs.llvm.org/show_bug.cgi?id=17101#c8

We could ease the one-u

[InstCombine] fold zext of masked bit set/clear

This does not solve PR17101, but it is one of the
underlying diffs noted here:
https://bugs.llvm.org/show_bug.cgi?id=17101#c8

We could ease the one-use checks for the 'clear'
(no 'not' op) half of the transform, but I do not
know if that asymmetry would make things better
or worse.

Proofs:
https://rise4fun.com/Alive/uVB

Name: masked bit set
%sh1 = shl i32 1, %y
%and = and i32 %sh1, %x
%cmp = icmp ne i32 %and, 0
%r = zext i1 %cmp to i32
=>
%s = lshr i32 %x, %y
%r = and i32 %s, 1

Name: masked bit clear
%sh1 = shl i32 1, %y
%and = and i32 %sh1, %x
%cmp = icmp eq i32 %and, 0
%r = zext i1 %cmp to i32
=>
%xn = xor i32 %x, -1
%s = lshr i32 %xn, %y
%r = and i32 %s, 1

show more ...

Revert "[InstCombine] Fix infinite loop due to bitcast <-> phi transforms"

This reverts commit 27a0795943fee0f30b995fe5165428afc2dfd402.

Seems to break test-suite.

[InstCombine] Fix infinite loop due to bitcast <-> phi transforms

Fix for https://bugs.llvm.org/show_bug.cgi?id=44245.

The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up
fighting against

[InstCombine] Fix infinite loop due to bitcast <-> phi transforms

Fix for https://bugs.llvm.org/show_bug.cgi?id=44245.

The optimizeBitCastFromPhi() and FoldPHIArgOpIntoPHI() end up
fighting against each other, because optimizeBitCastFromPhi()
assumes that bitcasts of loads will get folded. This doesn't happen
here, because a dangling phi node prevents the one-use fold in
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp#L620-L628 from triggering.

This patch fixes the issue by adding manually removing the old phis.

Differential Revision: https://reviews.llvm.org/D71164

show more ...

[InstCombine] Don't rewrite phi-of-bitcast when the phi has other users

Judging by the existing comments, this was the intention, but the
transform never actually checked if the existing phi's would

[InstCombine] Don't rewrite phi-of-bitcast when the phi has other users

Judging by the existing comments, this was the intention, but the
transform never actually checked if the existing phi's would be removed.
See https://bugs.llvm.org/show_bug.cgi?id=44242 for an example where
this causes much worse code generation on AMDGPU.

Differential Revision: https://reviews.llvm.org/D71209

show more ...

[InstCombine] Insert instructions before adding them to worklist

Summary:
This patch adds instructions to the InstCombine worklist after they are properly inserted. This way we don't get `<badref>`s

[InstCombine] Insert instructions before adding them to worklist

Summary:
This patch adds instructions to the InstCombine worklist after they are properly inserted. This way we don't get `<badref>`s printed when logging added instructions.
It also adds a check in `Worklist::Add` that ensures that all added instructions have parents.

Simple test case that illustrates the difference when run with `--debug-only=instcombine`:

```
define i32 @test35(i32 %a, i32 %b) {
%1 = or i32 %a, 1135
%2 = or i32 %1, %b
ret i32 %2
}
```

Before this patch:
```
INSTCOMBINE ITERATION #1 on test35
IC: ADDING: 3 instrs to worklist
IC: Visiting: %1 = or i32 %a, 1135
IC: Visiting: %2 = or i32 %1, %b
IC: ADD: %2 = or i32 %a, %b
IC: Old = %3 = or i32 %1, %b
New = <badref> = or i32 %2, 1135
IC: ADD: <badref> = or i32 %2, 1135
...
```

With this patch:
```
INSTCOMBINE ITERATION #1 on test35
IC: ADDING: 3 instrs to worklist
IC: Visiting: %1 = or i32 %a, 1135
IC: Visiting: %2 = or i32 %1, %b
IC: ADD: %2 = or i32 %a, %b
IC: Old = %3 = or i32 %1, %b
New = <badref> = or i32 %2, 1135
IC: ADD: %3 = or i32 %2, 1135
...
```

Reviewers: fhahn, davide, spatel, foad, grosser, nikic

Reviewed By: nikic

Subscribers: nikic, lebedev.ri, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71093

show more ...

Reland [DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. However, there were still pla

Reland [DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. However, there were still places
in which getIndexSizeInBits was used interchangeably with getPointerSizeInBits. This notably caused issues
with Instcombine's visitPtrToInt; but the unit tests was incorrect, so this remained undiscovered.

This fixes the buildbot failures.

Differential Revision: https://reviews.llvm.org/D68328

Patch by Joseph Faulls!

show more ...

Temporarily Revert "[DataLayout] Fix occurrences that size and range of pointers are assumed to be the same."

This reverts commit 5f6208778ff92567c57d7c1e2e740c284d7e69a5.

This caused failures in T

Temporarily Revert "[DataLayout] Fix occurrences that size and range of pointers are assumed to be the same."

This reverts commit 5f6208778ff92567c57d7c1e2e740c284d7e69a5.

This caused failures in Transforms/PhaseOrdering/scev-custom-dl.ll
const: Assertion `getBitWidth() == CR.getBitWidth() && "ConstantRange types don't agree!"' failed.

show more ...

[DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. However, there were still places
in

[DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. However, there were still places
in which getIndexSizeInBits was used interchangeably with getPointerSizeInBits. This notably caused issues
with Instcombine's visitPtrToInt; but the unit tests was incorrect, so this remained undiscovered.

Differential Revision: https://reviews.llvm.org/D68328

Patch by Joseph Faulls!

show more ...

Revert "[InstCombine] reduce code duplication; NFC"

This reverts commit db5739658467e20a52f20e769d3580412e13ff87.
At least 1 of these supposedly NFC commits wasn't - sanitizer bot is angry.

Revert "[InstCombine] improve readability; NFC"

This reverts commit 7250ef3613cc6b81145b9543bafb86d7f9466cde.
At least 1 of these supposedly NFC commits wasn't - sanitizer bot is angry.

Revert "[InstCombine] reduce indentation; NFC"

This reverts commit 8bf8ef7116bd0daec570b35480ca969b74e66c6e.
At least 1 of these supposedly NFC commits wasn't - sanitizer bot is angry.

[InstCombine] reduce indentation; NFC

[InstCombine] improve readability; NFC

CreateIntCast returns the input if its type matches, so need to duplicate that check.