111 Commits

Author SHA1 Message Date
Ralender
30c61119b9
[MergeICmps] Fix miss-compile in MergeICmps in presence of blockaddresses (#145925) 2025-06-26 22:32:31 +02:00
Nikita Popov
2d209d964a
[IR] Add getDataLayout() helpers to BasicBlock and Instruction (#96902)
This is a helper to avoid writing `getModule()->getDataLayout()`. I
regularly try to use this method only to remember it doesn't exist...

`getModule()->getDataLayout()` is also a common (the most common?)
reason why code has to include the Module.h header.
2024-06-27 16:38:15 +02:00
Yingwei Zheng
2f1f6b704d
[LLVM] Use std::move for APInt. NFC. (#86257)
This patch adjusts argument passing for `APInt` to improve the
compile-time.
Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=d1f182c895728d89c5c3d198b133e212a5d9d4a3&to=ba3e326def3a6e5cd6d72ff5a49c74fba18de1df&stat=instructions:u
2024-03-23 14:58:25 +08:00
Jeremy Morse
4427407a29 [NFC][RemoveDIs] Create a new spelling of the moveBefore method
As outlined in my proposal of how to get rid of debug intrinsics, this
patch adds a moveBefore method that signals the caller /intends/ the order
of moved instructions is to stay the same. This semantic difference has an
effect on debug-info, as it signals whether debug-info needs to move with
instructions or not.

The patch just replaces a few calls to moveBefore with calls to
moveBeforePreserving -- and the latter just calls the former, so it's all
NFC right now. A future patch will add an implementation of
moveBeforePreserving that takes action to correctly preserve debug-info,
but that's tightly coupled with our non-instruction debug-info
representation that's still being reviewed.

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939

Differential Revision: https://reviews.llvm.org/D156369
2023-09-07 18:37:57 +01:00
Elliot Goodrich
39d8e6e22c Add missing StringExtras.h includes
In preparation for removing the `#include "llvm/ADT/StringExtras.h"`
from the header to source file of `llvm/Support/Error.h`, first add in
all the missing includes that were previously included transitively
through this header.

This is fixing all files missed in b0abd4893fa1.

Differential Revision: https://reviews.llvm.org/D154543
2023-07-08 10:19:07 +01:00
Mikhail Goncharov
df3a8f3760 Revert "Reland [MergeICmps] Adapt to non-eq comparisons, bugfix"
Causes miscompile. See https://reviews.llvm.org/D141188.

This reverts commit fb2c98a929aa65603e9d984307a41325e577e9d3
2023-06-06 16:26:52 +02:00
Jie Fu
a3cc9d19b5 [MergeICmps] Fix -Wsign-compare and typos (NFC)
/data/llvm-project/llvm/lib/Transforms/Scalar/MergeICmps.cpp:623:21: error: comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigne
d long') [-Werror,-Wsign-compare]
  for (int i = 0; i < Comparisons.size(); i++) {
                  ~ ^ ~~~~~~~~~~~~~~~~~~
1 error generated.
2023-05-24 22:23:06 +08:00
Zhongyunde
fb2c98a929 Reland [MergeICmps] Adapt to non-eq comparisons, bugfix
1.Fix the last runtime issue as some sequent comparisons need be spilted.
For the origin equal comparisons chain, the new spilted Icmp chain will
still be end with equal, while for the new not-equal comparisons chain,
the new spilted Icmp chain will still be end with equal, so should address
this carefully, see detail wih case partial_sequent_ne

2. Fix the mismatch of last link comparison

Thanks for @aeubanks, @glandium and @ayzhao report the runtime issue
and carefully examine.
Fix https://github.com/llvm/llvm-project/issues/59740.

Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D141188
2023-05-24 22:06:23 +08:00
Arthur Eubanks
a5595e9f1f Revert "[MergeICmps] Adapt to non-eq comparisons, bugfix"
This reverts commit ae337ed5951c896164e07618d651d086f978ff2c.

Still causes miscompiles, see D141188.
2023-05-15 17:45:15 -07:00
Zhongyunde
ae337ed595 [MergeICmps] Adapt to non-eq comparisons, bugfix
Fix the last runtime issue as some sequent comparisons need be spilted.
For the origin equal comparisons chain, the new spilted Icmp chain will
still be end with equal, while for the new not-equal comparisons chain,
the new spilted Icmp chain will still be end with equal, so should address
this carefully, see detail wih case partial_sequent_ne

Thanks for @aeubanks, @glandium and @ayzhao report the runtime issue
and carefully examine.
Fix https://github.com/llvm/llvm-project/issues/59740.

Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D141188
2023-05-08 10:05:41 -07:00
Arthur Eubanks
3db8ae1f68 Revert "[MergeICmps] Adapt to non-eq comparisons, bugfix"
This reverts commit ca94b02e559242e6d1fcdd65320334438be69448.

Causes miscompiles, see D141188
2023-04-27 11:46:36 -07:00
Zhongyunde
ca94b02e55 [MergeICmps] Adapt to non-eq comparisons, bugfix
Fix the last runtime issue as some sequent comparisons need be spilted.
For the origin equal comparisons chain, the new spilted Icmp chain will
still be end with equal, while for the new not-equal comparisons chain,
the new spilted Icmp chain will still be end with equal, so should address
this carefully, see detail wih case partial_sequent_ne

Thanks for @aeubanks, @glandium and @ayzhao report the runtime issue
and carefully examine.
Fix https://github.com/llvm/llvm-project/issues/59740.

Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D141188
2023-04-25 19:41:04 +08:00
Zhongyunde
0e739ddd17 [MergeICmps] Attach metadata to new created loads
Use clone to keep the metadata, the issue is reported
by aeubanks on D141188.

Reviewed By: nikic, paulwalker-arm

Differential Revision: https://reviews.llvm.org/D146702
2023-04-08 10:45:58 +08:00
Krzysztof Drewniak
916425b2d1 [llvm] Use pointer index type for more GEP offsets (pre-codegen)
Many uses of getIntPtrType() were using that type to calculate the
neened type for GEP offset arguments. However, some time ago,
DataLayout was extended to support pointers where the size of the
pointer is not equal to the size of the values used to index it.

Much code was already migrated to, for example, use getIndexSizeInBits
instead of getPtrSizeInBits, but some rewrites still used
getIntPtrType() to get the type for GEP offsets.

This commit changes uses of getIntPtrType() to getIndexType() where
they are involved in a GEP-related calculation.

In at least one case (bounds check insertion) this resolves a compiler
crash that the new test added here would previously trigger.

This commit does not impact
- C library-related rewriting (memcpy()), which are operating under
the assumption that intptr_t == size_t. While all the mechanisms for
breaking this assumption now exist, doing so is outside the scope of
this commit.
- Code generation and below. Note that the use of getIntPtrType() in
CodeGenPrepare will be changed in a future commit.
- Usage of getIntPtrType() in any backend

Depends on D143435

Reviewed By: arichardson

Differential Revision: https://reviews.llvm.org/D143437
2023-03-28 16:41:02 +00:00
Arthur Eubanks
1929aa8f19 Revert "[MergeICmps] Adapt to non-eq comparisons, fix bug for cases need be spilted"
This reverts commit 818e554e251c1e07f133aeed9fe0473502ebfdae.

Causes miscompiles, see comments on D141188.
2023-03-10 13:25:49 -08:00
zhongyunde
818e554e25 [MergeICmps] Adapt to non-eq comparisons, fix bug for cases need be spilted
Fix the last runtime issue as some sequent comparisons need be spilted.
For the origin equal comparisons chain, the new spilted Icmp chain will
still be end with equal, while for the new not-equal comparisons chain,
the new spilted Icmp chain will still be end with equal, so should address
this carefully, see detail wih case partial_sequent_ne.

Thanks for @glandium and @ayzhao report the runtime issue and carefully
examine.
Fix https://github.com/llvm/llvm-project/issues/59740.

Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D141188
2023-03-09 23:49:09 +08:00
Zhongyunde
af2969fd46 Revert "[MergeICmps] Adapt to non-eq comparisons, retry"
This reverts commit 74ad19c25d7217d8f580a21d12fd4c784a1a0094.

test unittests/ProfileData/ProfileDataTests fails when built with
optimisations level -O1 with clang including this patch.
2023-03-02 09:07:44 +08:00
zhongyunde
74ad19c25d [MergeICmps] Adapt to non-eq comparisons, retry
Fix https://github.com/llvm/llvm-project/issues/59740.
NOTE: retry as we can't reproduce the break locally when first commit.

Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D141188
2023-02-27 01:26:13 +08:00
Vitaly Buka
46a8d5779b Revert "[MergeICmps] Adapt to non-eq comparisons"
Breaks ubsan build, details in D141188.

This reverts commit 3ac2b3a4f9effc9f79822e770f209fd70ff66362.
2023-01-12 18:14:47 -08:00
zhongyunde
3ac2b3a4f9 [MergeICmps] Adapt to non-eq comparisons
Fix https://github.com/llvm/llvm-project/issues/59740.

Reviewed By: courbet, nikic
Differential Revision: https://reviews.llvm.org/D141188
2023-01-12 09:47:02 +08:00
Fangrui Song
3152156334 [Transforms/Scalar] llvm::Optional => std::optional 2022-12-13 08:05:14 +00:00
Kazu Hirata
343de6856e [Transforms] Use std::nullopt instead of None (NFC)
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated.  The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
2022-12-02 21:11:37 -08:00
Fraser Cormack
cfb0d628a7 [MergeICmps][NFC] Fix a couple of typos in a comment 2022-11-15 14:46:23 +00:00
Bjorn Pettersson
aa1b64cc42 [BuildLibCalls] Use TLI to get 'int' and 'size_t' type sizes
Stop assuming that an 'int' is 32 bits in helpers that emit libcalls
to lib functions that had 'int' in the signature. For most targets
this is NFC. For a target with 16 bit 'int' type this could help out
detecting if trying to emit a libcall with incorrect signature.

Similarly we now derive the type mapping to 'size_t' by asking TLI
about the size of 'size_t'. This should be NFC (at least for in-tree
targets) since getSizeTSize(), in TLI, is deriving the size in the
same way as DataLayout::getIntPtrType().

Differential Revision: https://reviews.llvm.org/D135065
2022-10-04 12:52:05 +02:00
Nikita Popov
d3a52089eb Reapply [MergeICmps] Don't require GEP
Recommit without changes over 53abe3ff66a54117308352d379837c7d3229f8d6,
which addressed the cause of the reported crash.

-----

With opaque pointers, the zero-offset load will generally not use
a GEP. Allow a direct load without GEP, which is treated the same
way as a zero-offset GEP.
2022-03-04 11:39:11 +01:00
Nikita Popov
53abe3ff66 [MergeICmp] Make instruction move robust against empty block (NFCI)
Use the overload that support moving into an empty block. I don't
think that this situation can occur right now, but it can happen
with the change from e7fb1c15cb85d748c1c4fdd5a2eb5613ec7bef1d,
and the test is derived from the issue reported there.
2022-03-04 11:15:08 +01:00
Arthur Eubanks
bc1574b495 Revert "[MergeICmps] Don't require GEP"
This reverts commit e7fb1c15cb85d748c1c4fdd5a2eb5613ec7bef1d.

Causes crashes, see https://reviews.llvm.org/rGe7fb1c15cb85d748c1c4fdd5a2eb5613ec7bef1d.
2022-03-03 15:01:39 -08:00
Nikita Popov
e7fb1c15cb [MergeICmps] Don't require GEP
With opaque pointers, the zero-offset load will generally not use
a GEP. Allow a direct load without GEP, which is treated the same
way as a zero-offset GEP.
2022-02-25 17:38:02 +01:00
Nikita Popov
3c0096a1d4 [MergeICmps] Don't call comesBefore() if in different blocks (PR53959)
Only call comesBefore() if the instructions are in the same block.
Otherwise make a conservative assumption.

Fixes https://github.com/llvm/llvm-project/issues/53959.
2022-02-22 12:27:20 +01:00
Nikita Popov
e4a1af3724 [MergeICmps] Remove unused NumMerged variable 2021-09-21 21:43:25 +02:00
Nikita Popov
f2fa6ad047 [MergeICmps] Don't reorder unmerged comparisons
MergeICmps will currently sort (by offset) all comparisons in a chain,
including those that do not get merged. This is problematic in two ways:

 * We may end up moving the original first block into the middle of
   the chain, in which case the "extra work" instructions will also
   be in the middle of the chain, resulting in invalid IR
   (reported in https://reviews.llvm.org/D108782#3005583).
 * Reordering branches is generally not legal, because it may
   introduce branch on poison, which is UB (PR51845). The merging
   done by MergeICmps is legal as long as we assume that memcmp()
   works on frozen memory, but the reordering of unmerged comparisons
   is definitely incorrect (without inserting freeze instructions),
   so we should avoid it.

There are easier ways to fix the first issue, but I figured it was
worthwhile to do this properly to also fix the second one. What we
now do is to restore the original relative order of (potentially
merged) comparisons.

I took the liberty of dropping the MERGEICMPS_DOT_ON functionality,
because it would be more awkward to implement now (as the before and
after representation is different) and it doesn't seem terribly
useful nowadays.

Differential Revision: https://reviews.llvm.org/D110024
2021-09-21 21:22:12 +02:00
Kazu Hirata
e2febc2ed4 [llvm] Use drop_begin (NFC) 2021-09-17 09:16:40 -07:00
Nikita Popov
757409da7a [MergeICmps] Ignore clobbering instructions before the loads
This is another followup to D106591. Even if there is an
instruction that clobbers one of the loads, this doesn't matter if
it happens before the loads. Those instructions aren't affected by
the transform at all.

The gep-references-bb.ll is modified to preserve the spirit of the
test, as the store to @g no longer impacts the transform.

Differential Revision: https://reviews.llvm.org/D108782
2021-08-27 23:31:35 +02:00
Valentin Churavy
4cacb5cad0
[MergeICmps] Don't merge icmps derived from pointers with addressspaces
IIUC we can't emit `memcmp` between pointers in addressspaces,
doing so will trigger an assertion since the signature of the memcmp
will not match it's arguments (https://bugs.llvm.org/show_bug.cgi?id=48661).

This PR disables the attempt to merge icmps,
when the pointer is in an addressspace.

Reviewed By: #julialang, vtjnash

Differential Revision: https://reviews.llvm.org/D94813
2021-08-27 22:15:02 +02:00
Nikita Popov
19dc02e99f [MergeICmps] Allow sinking past non-load/store
This is a followup to D106591. MergeICmps currently only allows
sinking the loads past either instructions that don't write to
memory at all, or simple loads/stores that don't modify the memory
the loads access.

The "simple loads/stores" part of this check doesn't seem necessary
to me -- AA isModRef() already accurately models any operation
that may clobber the memory. For example, in the adjusted test case
the transform is still fine if the call to @foo() isn't readonly,
but inaccessiblememonly -- in both cases, the call cannot modify
the loaded memory.

Differential Revision: https://reviews.llvm.org/D108517
2021-08-23 22:03:49 +02:00
Nikita Popov
f921bf6049 [MergeICmps] Collect block instructions once (NFC)
Collect the relevant instructions for a given BCECmpBlock once
on construction, rather than repeating this logic in multiple
places.
2021-07-26 18:07:20 +02:00
Nikita Popov
c691651c53 [MergeICmps] Try to fix MSVC build failure
Apparently this fails to line up the types -- try to sidestep the
issue entirely by writing the code in a more reasonable way: Walk
over the operands and perform a set lookup, rather than walking
over the set and performing an operand scan.
2021-07-26 17:31:27 +02:00
Nikita Popov
0d3807b365 [MergeICmps] Separate out BCECmp and use Optional (NFC)
Separate out the BCECmp part from BCECmpBlock, which just stores
the comparison atoms without the branch instruction. At the same
time switch the code to return Optional<> rather than objects in
invalid state and partially constructed objects.
2021-07-26 17:06:43 +02:00
Nikita Popov
f502683750 [MergeICmps] Relax sinking check
The check for sinking instructions past the load + cmp sequence
currently checks for side-effects, which includes writing to memory
and unwinding. However, I don't believe we care about sinking the
instructions past an unwind (as they don't have any side-effects
themselves).

Differential Revision: https://reviews.llvm.org/D106591
2021-07-23 22:16:11 +02:00
David Blaikie
1def2579e1 PR51018: Remove explicit conversions from SmallString to StringRef to future-proof against C++23
C++23 will make these conversions ambiguous - so fix them to make the
codebase forward-compatible with C++23 (& a follow-up change I've made
will make this ambiguous/invalid even in <C++23 so we don't regress
this & it generally improves the code anyway)
2021-07-08 13:37:57 -07:00
Arthur Eubanks
6b9524a05b [NewPM] Don't mark AA analyses as preserved
Currently all AA analyses marked as preserved are stateless, not taking
into account their dependent analyses. So there's no need to mark them
as preserved, they won't be invalidated unless their analyses are.

SCEVAAResults was the one exception to this, it was treated like a
typical analysis result. Make it like the others and don't invalidate
unless SCEV is invalidated.

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D102032
2021-05-18 13:49:03 -07:00
Nikita Popov
fb9ed1979a [IR] Add BasicBlock::isEntryBlock() (NFC)
This is a recurring and somewhat awkward pattern. Add a helper
method for it.
2021-05-15 12:41:58 +02:00
Timm Bäder
e60d6e91e1 [llvm][NFC] Fix assert indentation
This triggers GCC's misleading-indentation checker.
2021-04-23 14:44:05 +02:00
Kazu Hirata
fb74e1e78a [Transforms/Scalar] Use range-based for loops (NFC) 2021-02-04 21:18:05 -08:00
Kazu Hirata
4d92ab1669 [Transforms] Use llvm::find_if (NFC) 2021-01-09 09:24:58 -08:00
Clement Courbet
735e6c888e [MergeICmps] Fix missing split.
We were not correctly splitting a blocks for chains of length 1.

Before that change, additional instructions for blocks in chains of
length 1 were not split off from the block before removing (this was
done correctly for chains of longer size).
If this first block contained an instruction referenced elsewhere,
deleting the block, would result in invalidation of the produced value.

This caused a miscompile which motivated D92297 (before D17993,
nonnull and dereferenceable attributed were not added so MergeICmps were
not triggered.) The new test gep-references-bb.ll demonstrate the issue.

The regression was introduced in
rG0efadbbcdeb82f5c14f38fbc2826107063ca48b2.

This supersedes D92364.

Test case by MaskRay (Fangrui Song).

Differential Revision: https://reviews.llvm.org/D92375
2020-12-01 16:50:55 +01:00
Simon Pilgrim
6aa10ae5bf [Transforms] visitCmpBlock - don't dereference a dyn_cast<>. NFCI.
Use cast<> as we immediately dereference the pointer afterwards - cast<> will assert if we fail.

Prevents clang static analyzer warning that we could deference a null pointer.
2020-10-08 20:18:32 +01:00
Reid Kleckner
05da2fe521 Sink all InitializePasses.h includes
This file lists every pass in LLVM, and is included by Pass.h, which is
very popular. Every time we add, remove, or rename a pass in LLVM, it
caused lots of recompilation.

I found this fact by looking at this table, which is sorted by the
number of times a file was changed over the last 100,000 git commits
multiplied by the number of object files that depend on it in the
current checkout:
  recompiles    touches affected_files  header
  342380        95      3604    llvm/include/llvm/ADT/STLExtras.h
  314730        234     1345    llvm/include/llvm/InitializePasses.h
  307036        118     2602    llvm/include/llvm/ADT/APInt.h
  213049        59      3611    llvm/include/llvm/Support/MathExtras.h
  170422        47      3626    llvm/include/llvm/Support/Compiler.h
  162225        45      3605    llvm/include/llvm/ADT/Optional.h
  158319        63      2513    llvm/include/llvm/ADT/Triple.h
  140322        39      3598    llvm/include/llvm/ADT/StringRef.h
  137647        59      2333    llvm/include/llvm/Support/Error.h
  131619        73      1803    llvm/include/llvm/Support/FileSystem.h

Before this change, touching InitializePasses.h would cause 1345 files
to recompile. After this change, touching it only causes 550 compiles in
an incremental rebuild.

Reviewers: bkramer, asbirlea, bollu, jdoerfert

Differential Revision: https://reviews.llvm.org/D70211
2019-11-13 16:34:37 -08:00
Dmitri Gribenko
2bf8d77453 Revert "Reland "r364412 [ExpandMemCmp][MergeICmps] Move passes out of CodeGen into opt pipeline.""
This reverts commit r371502, it broke tests
(clang/test/CodeGenCXX/auto-var-init.cpp).

llvm-svn: 371507
2019-09-10 10:39:09 +00:00
Clement Courbet
612c260ec3 Reland "r364412 [ExpandMemCmp][MergeICmps] Move passes out of CodeGen into opt pipeline."
With a fix for sanitizer breakage (see explanation in D60318).

llvm-svn: 371502
2019-09-10 09:18:00 +00:00