[all-commits] [llvm/llvm-project] 497829: [ARM, MVE] Support -ve offsets in gather-load intri...

[Simon Tatham via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 4978296cd8e4d10724cfa41f0308d256c0fd490c
      https://github.com/llvm/llvm-project/commit/4978296cd8e4d10724cfa41f0308d256c0fd490c
  Author: Simon Tatham <simon.tatham at arm.com>
  Date:   2020-01-06 (Mon, 06 Jan 2020)

  Changed paths:
    M clang/include/clang/Basic/arm_mve_defs.td
    M clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
    M clang/test/Sema/arm-mve-immediates.c
    M clang/utils/TableGen/MveEmitter.cpp
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll

  Log Message:
  -----------
  [ARM,MVE] Support -ve offsets in gather-load intrinsics.

Summary:
The ACLE intrinsics with `gather_base` or `scatter_base` in the name
are wrappers on the MVE load/store instructions that take a vector of
base addresses and an immediate offset. The immediate offset can be up
to 127 times the alignment unit, and it can be positive or negative.

At the MC layer, we got that right. But in the Sema error checking for
the wrapping intrinsics, the offset was erroneously constrained to be
positive.

To fix this I've adjusted the `imm_mem7bit` class in the Tablegen that
defines the intrinsics. But that causes integer literals like
`0xfffffffffffffe04` to appear in the autogenerated calls to
`SemaBuiltinConstantArgRange`, which provokes a compiler warning
because that's out of the non-overflowing range of an `int64_t`. So
I've also tweaked `MveEmitter` to emit that as `-0x1fc` instead.

Updated the tests of the Sema checks themselves, and also adjusted a
random sample of the CodeGen tests to actually use negative offsets
and prove they get all the way through code generation without causing
a crash.

Reviewers: dmgreen, miyuki, MarkMurrayARM

Reviewed By: dmgreen

Subscribers: kristof.beyls, cfe-commits, llvm-commits

Tags: #clang, #llvm

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


  Commit: 34817e04feeb00dcd0515e5810218587438bd5a8
      https://github.com/llvm/llvm-project/commit/34817e04feeb00dcd0515e5810218587438bd5a8
  Author: Simon Tatham <simon.tatham at arm.com>
  Date:   2020-01-06 (Mon, 06 Jan 2020)

  Changed paths:
    M clang/include/clang/Basic/arm_mve.td
    M clang/test/CodeGen/arm-mve-intrinsics/vabdq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vhaddq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vhsubq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vmaxnmq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vmaxq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vminnmq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vminq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vmulhq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vmullbq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vmulltq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vqaddq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vqsubq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vrhaddq.c
    M clang/test/CodeGen/arm-mve-intrinsics/vrmulhq.c
    M llvm/include/llvm/IR/IntrinsicsARM.td
    M llvm/lib/Target/ARM/ARMInstrMVE.td
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vabdq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vhaddq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vhsubq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vmaxnmq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vmaxq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vminnmq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vminq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vmulhq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vmullbq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vmulltq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vqaddq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vqsubq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vrhaddq.ll
    M llvm/test/CodeGen/Thumb2/mve-intrinsics/vrmulhq.ll

  Log Message:
  -----------
  [ARM,MVE] Fix many signedness errors in MVE intrinsics.

Summary:
Running an end-to-end test last week I noticed that a lot of the ACLE
intrinsics that operate differently on vectors of signed and unsigned
integers were ending up generating the signed version of the
instruction unconditionally. This is because the IR intrinsics had no
way to distinguish signed from unsigned: the LLVM type system just
calls them both `v8i16` (or whatever), so you need either separate
intrinsics for signed and unsigned, or a flag parameter that tells
ISel which one to choose.

This patch fixes all the problems of that kind that I've noticed, by
adding an i32 flag parameter to many of the IR intrinsics which is set
to 1 for unsigned (matching the existing practice in cases where we
got it right), and conditioning all the isel patterns on that flag. So
the fundamental change is in `IntrinsicsARM.td`, changing the
low-level IR intrinsics API; there are knock-on changes in
`arm_mve.td` (adjusting code gen for the ACLE intrinsics to use the
modified API) and in `ARMInstrMVE.td` (adjusting isel to expect the
new unsigned flags). The rest of this patch is boringly updating tests.

Reviewers: dmgreen, miyuki, MarkMurrayARM

Reviewed By: dmgreen

Subscribers: kristof.beyls, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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


Compare: https://github.com/llvm/llvm-project/compare/b99ef32d041c...34817e04feeb