[all-commits] [llvm/llvm-project] e02422: [lld][RISCV][test] Expand testing in riscv-attribu...

Mon Feb 27 00:15:09 PST 2023

  Branch: refs/heads/release/16.x
  Home:   https://github.com/llvm/llvm-project
  Commit: e0242291aaa9d5bc619781dad7e1191769c73726
      https://github.com/llvm/llvm-project/commit/e0242291aaa9d5bc619781dad7e1191769c73726
  Author: Alex Bradbury <asb at igalia.com>
  Date:   2023-02-27 (Mon, 27 Feb 2023)

  Changed paths:
    M lld/test/ELF/riscv-attributes.s

  Log Message:
  -----------
  [lld][RISCV][test] Expand testing in riscv-attributes.s

This patch is related to the discussion in
<https://discourse.llvm.org/t/rfc-resolving-issues-related-to-extension-versioning-in-risc-v/68472/1>.
It slightly expands the coverage of behaviour for unrecognized
extensions (there are slightly different forms of error for zfoo vs e.g.
'y'), and adds coverage for an extension with an
unrecognized/unsupported version number.

Use "unrecognized" terminology because the expectation is the error
checking will be reduced in a follow-up patch posted for review.

(cherry picked from commit 8b5004864aab71859e961af02eb1a5ceeae887f7)

  Commit: ab91ea89b60e8708e5b9b4fcfc6b94c42e8bd02b
      https://github.com/llvm/llvm-project/commit/ab91ea89b60e8708e5b9b4fcfc6b94c42e8bd02b
  Author: Alex Bradbury <asb at igalia.com>
  Date:   2023-02-27 (Mon, 27 Feb 2023)

  Changed paths:
    M lld/test/ELF/riscv-attributes-place.s
    M lld/test/ELF/riscv-attributes.s

  Log Message:
  -----------
  [lld][test][RISCV] Don't use incorrectly normalised arch string in riscv-attributes-place.s

Per the psABI, the arch string should be normalised to (amongest other
things) always include the full version of each extension in form
zfoo1p0. riscv-attributes-place.s didn't conform to this, which is not a
problem for the current parsing logic, but this behaviour would change
with a patch I'm about to propose.

This makes riscv-sttributes-place.s feature a valid arch string, and
maintains test coverage for this particular form of invalid arch string
by adding it to riscv-attributes.s.

(cherry picked from commit 179a24c2f149933868e2a69b94200d7f4dcf18c5)

  Commit: d42c23266d441a3e9d5a2dcb00c3a238fb67eb75
      https://github.com/llvm/llvm-project/commit/d42c23266d441a3e9d5a2dcb00c3a238fb67eb75
  Author: Alex Bradbury <asb at igalia.com>
  Date:   2023-02-27 (Mon, 27 Feb 2023)

  Changed paths:
    M lld/ELF/Arch/RISCV.cpp
    M lld/test/ELF/riscv-attributes.s
    M llvm/include/llvm/Support/RISCVISAInfo.h
    M llvm/lib/Support/RISCVISAInfo.cpp
    M llvm/unittests/Support/CMakeLists.txt
    A llvm/unittests/Support/RISCVISAInfoTest.cpp

  Log Message:
  -----------
  [lld][RISCV] Avoid error when encountering unrecognised ISA extensions/versions in RISC-V attributes

This patch follows on from this RFC thread
<https://discourse.llvm.org/t/rfc-resolving-issues-related-to-extension-versioning-in-risc-v/68472/>
and the ensuing discussion in the RISC-V LLVM sync-up call. The
consensus agreed that the behaviour change in LLD introduced in D138550
that results in object files including arch attributes with unrecognised
extensions or versions of extensions is a regression and should be
treated as such. As it stands, this logic means that LLD will error out
if trying to link a RISC-V object file from LLVM HEAD (rv32i2p0/rv64i2p0)
with one from current GCC (rv32i2p1/rv64i2p1 by default).

There's a bigger discussion about exactly when to warn vs error and so
on, and how to control that, and this patch doesn't attempt to address
all those questions. It simply tries to fix the problem with a minimally
invasive change, intended to be cherry-picked for 16.0.x (ideally
16.0.0, but queued for 16.0.1 if we're too late in the release cycle).

As you can see from the test changes, although the changed logic is
mostly more permissive, it will reject some embedded arch strings that
were accepted before. Because the same logic was previously used for
parsing human-written -march as for the arch attribute (intended to be
stored in normalised form), various short-hand formats were previously
accepted. Maintaining support for such ill-formed attributes would
substantially complicate the logic, and given the previous
implementation was so much stricter in every other way, was surely a bug
rather than a design choice.

Surprisingly, the precise rules for how numbers can be embedded into
extension names isn't fully defined (there is a PR to the ISA manual
that is not yet merged
<https://github.com/riscv/riscv-isa-manual/pull/718>).
In the absence of specific criteria for rejecting extensions names that
would be ambiguous in abbreviated form,
`RISCVISAInfo::parseArchStringNormalized` just pulls out the version
information from the end of each extension description.

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

(cherry picked from commit ff93c9beefd4848c6a482ab57d6667eb40344026)

  Commit: abf32977809e8a04c9c98b9f3c72d43e37931ee9
      https://github.com/llvm/llvm-project/commit/abf32977809e8a04c9c98b9f3c72d43e37931ee9
  Author: Florian Hahn <flo at fhahn.com>
  Date:   2023-02-27 (Mon, 27 Feb 2023)

  Changed paths:
    M llvm/include/llvm/Analysis/ScalarEvolution.h

  Log Message:
  -----------
  [SCEV] Increase FoldID bits size cover common cases.

This should fix the regressions introduced by a53d940cee6f by making
sure no dynamic allocations are needed in the common case, while
retaining support for arbitrary SCEV expressions.

Alternative to D144335.

Compile-time impact:
* NewPM-O3: -0.11%
* NewPM-ReleaseThinLTO: -0.10%
*NewPM-ReleaseLTO-g: -0.08%

https://llvm-compile-time-tracker.com/compare.php?from=df016a9525e5722dfd1e1e1632cec3ed7b33bc8a&to=c1c64de4a973bcecaddbc4038a539234eb39413b&stat=instructions:u

Reviewed By: vitalybuka, nikic

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

(cherry picked from commit eddecd3ade5ce1b827dafb84e8137cc6f89576fb)

Compare: https://github.com/llvm/llvm-project/compare/e3ce92b7a904...abf32977809e