Skip to content

Conversation

@pfez
Copy link
Contributor

@pfez pfez commented Jan 7, 2021

This patchset rebases the out-of-tree commits we have (previosly based on top of llvm-10) on top of llvm-11.

This is the first step for upgrading all revng to llvm-11. More PRs will come on other projects, and they should all be merged together to avoid breaking things.

nemanjai and others added 30 commits July 27, 2020 16:26
I mixed up the precedence of operators in the assert and thought I
had it right since there was no compiler warning. This just
adds the parentheses in the expression as needed.

(cherry picked from commit cdead4f)
…irect branch (PR46857)

SplitBlockPredecessors() can not split blocks that have such terminators,
and in two other places we already ensure that we don't end up calling
SplitBlockPredecessors() on such blocks. Do so in one more place.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46857

(cherry picked from commit 1da9834)
Previously, the test could pass with one part of c3b1d73 removed.

(cherry picked from commit 8dc8203)
… different name

For a weak symbol func in a comdat, the actual leader symbol ends up
named like .weak.func.default*. Likewise, for stdcall on i386, the symbol
may be named _func@4, while the section suffix only is "func", which the
previous implementation didn't handle.

This fixes unwinding through weak functions when using
-ffunction-sections in mingw environments.

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

(cherry picked from commit 343ffa7)
As shown in D82998, the basic-aa-recphi option can cause miscompiles for
gep's with negative constants. The option checks for recursive phi, that
recurse through a contant gep. If it finds one, it performs aliasing
calculations using the other phi operands with an unknown size, to
specify that an unknown number of elements after the initial value are
potentially accessed. This works fine expect where the constant is
negative, as the size is still considered to be positive. So this patch
expands the check to make sure that the constant is also positive.

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

(cherry picked from commit 311fafd)
…it as livein to the basic blocks created when expanding the pseudo

XBEGIN causes several based blocks to be inserted. If flags are live across it we need to make eflags live in the new basic blocks to avoid machine verifier errors.

Fixes PR46827

Reviewed By: ivanbaev

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

(cherry picked from commit 647e861)
…HOP(X,Y))

An initial backend patch towards fixing the various poor HADD combines (PR34724, PR41813, PR45747 etc.).

This extends isHorizontalBinOp to check if we have per-element horizontal ops (odd+even element pairs), but not in the expected serial order - in which case we build a "post shuffle mask" that we can apply to the HOP result, assuming we have fast-hops/optsize etc.

The next step will be to extend the SHUFFLE(HOP(X,Y)) combines as suggested on PR41813 - accepting more post-shuffle masks even on slow-hop targets if we can fold it into another shuffle.

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

(cherry picked from commit 1821117)
Previously this flag was just ignored. If set, set the
IMAGE_DLL_CHARACTERISTICS_NO_SEH bit, regardless of the normal safeSEH
machinery.

In mingw configurations, the safeSEH bit might not be set in e.g. object
files built from handwritten assembly, making it impossible to use the
normal safeseh flag. As mingw setups don't generally use SEH on 32 bit
x86 at all, it should be fine to set that flag bit though - hook up
the existing GNU ld flag for controlling that.

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

(cherry picked from commit 745eb02)
The previous fix for this, https://reviews.llvm.org/D76761, Passed test cases but failed in the real world as std::string has a non trivial destructor so creates a CXXBindTemporaryExpr.

This handles that shortfall and updates the test case std::basic_string implementation to use a non trivial destructor to reflect real world behaviour.

Reviewed By: gribozavr2

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

(cherry picked from commit b99630e)
v3i16 and v3f16 currently cannot be legalized and lowered so they should
not be emitted by inst combining.

Moved the check down to still allow extracting 1 or 2 elements via the dmask.

Fixes image intrinsics being combined to return v3x16.

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

(cherry picked from commit 2c65908)
This isn't a natively supported operation, so convert it to a
mask+compare.

In addition to the operation itself, fix up some surrounding stuff to
make the testcase work: we need concat_vectors on i1 vectors, we need
legalization of i1 vector truncates, and we need to fix up all the
relevant uses of getVectorNumElements().

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

(cherry picked from commit b8f765a)
The default calling convention needs to save/restore the SVE callee
saves according to the SVE PCS when the function takes or returns
scalable types, even when the `aarch64_sve_vector_pcs` CC is not
specified for the function.

Reviewers: efriedma, paulwalker-arm, david-arm, rengolin

Reviewed By: paulwalker-arm

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

(cherry picked from commit 9bacf15)
This patch addresses two issues:

* Forces the availability of the base-pointer (x19) when the frame has
  both scalable vectors and variable-length arrays. Otherwise it will
  be expensive to access non-SVE locals.

* In presence of SVE stack objects, it will allocate the emergency
  scavenging slot close to the SP, so that they can be accessed from
  the SP or BP if available. If accessed from the frame-pointer, it will
  otherwise need an extra register to access the scavenging slot because
  of mixed scalable/non-scalable addressing modes.

Reviewers: efriedma, ostannard, cameron.mcinally, rengolin, david-arm

Reviewed By: efriedma

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

(cherry picked from commit bef56f7)
It's sort of tricky to hit this in practice, but not impossible. I have
a synthetic C testcase if anyone is interested.

The implementation is identical to the equivalent NEON register copies.

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

(cherry picked from commit 993c1a3)
Fixed stack objects are preallocated and defined to be allocated before
any of the regular stack objects. These are normally used to model stack
arguments.

The AAPCS does not support passing SVE registers on the stack by value
(only by reference). The current layout also doesn't place them before
all stack objects, but rather before all SVE objects. Removing this
simplifies the code that emits the allocation/deallocation
around callee-saved registers (D84042).

This patch also removes all uses of fixedStack from from
framelayout-sve.mir, where this was used purely for testing purposes.

Reviewers: paulwalker-arm, efriedma, rengolin

Reviewed By: paulwalker-arm

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

(cherry picked from commit 54492a5)
Instead of aligning the last callee-saved-register slot to the stack
alignment (16 bytes), just align the SVE callee-saved block. This also
simplifies the code that allocates space for the callee-saves.

This change is needed to make sure the offset to which the callee-saved
register is spilled, corresponds to the offset used for e.g. unwind call
frame instructions.

Reviewers: efriedma, paulwalker-arm, david-arm, rengolin

Reviewed By: efriedma

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

(cherry picked from commit 26b4ef3)
While deallocating the stackframe, the offset used to reload the
callee-saved registers was not pointing to the SVE callee-saves,
but rather to the whole SVE area.

   +--------------+
   | GRP callee   |
   |     saves    |
   +--------------+ <- FP
   | SVE callee   |
   |     saves    |
   +--------------+ <- Should restore SVE callee saves from here
   |  SVE Spills  |
   |  and Locals  |
   +--------------+ <- instead of from here.
   |              |
   :              :
   |              |
   +--------------+ <- SP

Reviewed By: paulwalker-arm

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

(cherry picked from commit cda2eb3)
I have introduced a new TargetFrameLowering query function:

  isStackIdSafeForLocalArea

that queries whether or not it is safe for objects of a given stack
id to be bundled into the local area. The default behaviour is to
always bundle regardless of the stack id, however for AArch64 this is
overriden so that it's only safe for fixed-size stack objects.
There is future work here to extend this algorithm for multiple local
areas so that SVE stack objects can be bundled together and accessed
from their own virtual base-pointer.

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

(cherry picked from commit 14bc85e)
…plitVecOp_EXTRACT_SUBVECTOR

In DAGTypeLegalizer::SplitVecOp_EXTRACT_SUBVECTOR I have replaced
calls to getVectorNumElements with getVectorMinNumElements, since
this code path works for both fixed and scalable vector types. For
scalable vectors the index will be multiplied by VSCALE.

Fixes warnings in this test:

  sve-sext-zext.ll

Differential revision: https://reviews.llvm.org/D83198

(cherry picked from commit 5d84eaf)
Previous patches fixed up all the warnings in this test:

  llvm/test/CodeGen/AArch64/sve-sext-zext.ll

and this change simply checks that no new warnings are added in future.

Differential revision: https://reviews.llvm.org/D83205

(cherry picked from commit f43b5c7)
I have added tests to:

  CodeGen/AArch64/sve-intrinsics-int-arith.ll

for doing simple integer add operations on tuple types. Since these
tests introduced new warnings due to incorrect use of
getVectorNumElements() I have also fixed up these warnings in the
same patch. These fixes are:

1. In narrowExtractedVectorBinOp I have changed the code to bail out
early for scalable vector types, since we've not yet hit a case that
proves the optimisations are profitable for scalable vectors.
2. In DAGTypeLegalizer::WidenVecRes_CONCAT_VECTORS I have replaced
calls to getVectorNumElements with getVectorMinNumElements in cases
that work with scalable vectors. For the other cases I have added
asserts that the vector is not scalable because we should not be
using shuffle vectors and build vectors in such cases.

Differential revision: https://reviews.llvm.org/D84016

(cherry picked from commit 2078771)
…orizeChainsInBlock

In vectorizeChainsInBlock we try to collect chains of PHI nodes
that have the same element type, but the code is relying upon
the implicit conversion from TypeSize -> uint64_t. For now, I have
modified the code to ignore PHI nodes with scalable types.

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

(cherry picked from commit 9ad7c98)
…th scalable types

When building code at -O0 We weren't falling back to DAG ISel correctly
when encountering alloca instructions with scalable vector types. This
is because the alloca has no operands that are scalable. I've fixed this by
adding a check in AArch64ISelLowering::fallBackToDAGISel for alloca
instructions with scalable types.

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

(cherry picked from commit 23ad660)
Reviewers: kmclaughlin, efriedma, sdesmalen

Subscribers: tschuett, hiraditya, psnobl, llvm-commits

Tags: #llvm

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

(cherry picked from commit 809600d)
andrealinux1 and others added 23 commits November 11, 2020 15:38
The destructor was virtual but not marked override, causing warnings
(and errors in revng if -Wall) was enabled.
This commit fixes these problems
`virtual` classes should always have a virtual destructor.
This commit improves the handling of comparision of a value masked in a
certain way with a constant. For example, for a 16-bit integer, we might
have x & 0xfff0 == 0x1230. This is a typical situation produced by
InstCombine to efficiently check if x is between 0x1230 and 0x12340
(excluded).
This silences warnings when compiling with clang-10 and -std=c++2a, that
complain about various ambiguities in comparison operators operator==
and operator!= of iterator classes that inherit with CRTP from
iterator_adaptor_base.

This new set of ambiguities is introduced by C++20 rules for rewriting
comparison operators, that are designed to reduce the number of
comparison that you typically have to write, allowing the compiler to
rewrite and reverse comparison operators, so that most comparison
operators can be rewritten in terms of the base ones.

Turns out this creates some problems with CRTP because comparison
operators of the CRPT base, that were fine until C++17, now become
ambiguous.

In particular, this is due to the fact that for C++20 operator== and
operator!= are supposed to be symmetric (LHS and RHS need to be
swappable), and having

  CRTPBase::operator==(const CRTPDerived &D) const;

screws things up, because the type of the operator parameter
(CRTPDerived) is not equal to the type of the class for which we are
implementing the operator (CRTPBase).

This forces us to add the following

bool CRTPDerived::operator==(const CRTPDerived &D) const {
  return CRTBase::operator==(D)
}

for most of the classes that inherit from iterator_adaptor_base with
CRTP, to stop the compiler from complaining.
Before this patch, LLVM's DominatorTree and related templates
(DominatorTree, DomTreeBuilder) did not support the computation of
dominator and post-dominator trees on graphs with markers, such as
GraphTraits<llvm::Inverse<T>>.
They only supported plain graph traits.

This patch restructures all the necessary templates, adding an
additional template argument (View), which allows to compute DT and PDT
on all the graphs that use GraphTraits, potentially equipped with a
View.
View is a marker, that allows to provide a different view on a graph
that has GraphTraits.
A typical example of this is llvm::Inverse.

This mechanism of View markers will be used in rev.ng to implement
filtered graph traits, to filter the edges of a graph implemented with
GraphTraits.
The new DT and PDT on the marked version of the GraphTraits will be
computed on the same graphs, but treating some edges as if they are seen
through the view.
For instance, the Post-Dominator Tree of a graph marked with
llvm::Inverse view is the Dominator Tree.

The previous default behavior of DT and PDT is implemented by means of
the DTIdentityView, which simply leaves the node type of the GraphTraits
untouched.
This is a transparent view that allows all the code in llvm to continue
working as before this patch.

This patch has been tested in the llvm-project monorepo with the
following targets:
 - make check-llvm-unit
 - make check-llvm
 - make check-clang-unit
 - make check-clang
@pfez pfez mentioned this pull request Jan 7, 2021
@pfez pfez marked this pull request as draft January 7, 2021 13:23
@pfez pfez closed this Jun 16, 2021
@pfez
Copy link
Contributor Author

pfez commented Jun 16, 2021

Obsoleted by #5

aleclearmind pushed a commit that referenced this pull request Apr 5, 2023
Change https://reviews.llvm.org/D140059 exposed the following crash in
Z3Solver, where bit widths were not checked consistently with that
change. This change makes the check consistent, and fixes the crash.

```
clang: <root>/llvm/include/llvm/ADT/APSInt.h:99:
  int64_t llvm::APSInt::getExtValue() const: Assertion
  `isRepresentableByInt64() && "Too many bits for int64_t"' failed.
...
Stack dump:
0. Program arguments: clang -cc1 -internal-isystem <root>/lib/clang/16/include
  -nostdsysteminc -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection
  -analyzer-config crosscheck-with-z3=true -verify reproducer.c

 #0 0x00000000045b3476 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int)
  <root>/llvm/lib/Support/Unix/Signals.inc:567:22
 #1 0x00000000045b3862 PrintStackTraceSignalHandler(void*)
  <root>/llvm/lib/Support/Unix/Signals.inc:641:1
 #2 0x00000000045b14a5 llvm::sys::RunSignalHandlers()
  <root>/llvm/lib/Support/Signals.cpp:104:20
 #3 0x00000000045b2eb4 SignalHandler(int)
  <root>/llvm/lib/Support/Unix/Signals.inc:412:1
 ...
 #9 0x0000000004be2eb3 llvm::APSInt::getExtValue() const
  <root>/llvm/include/llvm/ADT/APSInt.h:99:5
  <root>/llvm/lib/Support/Z3Solver.cpp:740:53
  clang::ASTContext&, clang::ento::SymExpr const*, llvm::APSInt const&, llvm::APSInt const&, bool)
  <root>/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:552:61
```

Reviewed By: steakhal

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

(cherry picked from commit f027dd55f32a3c1803fc3cbd53029acee849465c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.