[PATCH] D112942: target ABI: improve call parameters extensions handling

Mon Nov 1 11:12:52 PDT 2021

jonpa created this revision.
jonpa added reviewers: uweigand, rnk.
Herald added subscribers: luke957, ormris, dexonsmith, jdoerfert, pengfei, s.egerton, steven_wu, simoncook, hiraditya, arichardson.
jonpa requested review of this revision.
Herald added a project: LLVM.

As discussed on llvm-dev (https://lists.llvm.org/pipermail/llvm-dev/2021-October/153080.html), there has been a recurring problem of missing extension attributes on call arguments.

This patch tries to improve on the situation by intorducing a 'NoExt' attribute that is (at least in theory, at the moment) mandatory for a struct passed in an integer register. By demanding that a (narrow) integer argument has one of the sign/zero/no extension attributes, cases where this has been forgotten in the FE can be isolated.

Changes:

- Clang FE:
  - A new bit ZeroExt in ABIArgInfo. If Kind is Extend, it is SignExt or ZeroExt, or else NoExt is implied.
  - A new method ABIArgInfo::getNoExtend(), used by SystemZABIInfo::classifyArgumentType() for a struct-in-reg argument. (CGCall.cpp:CodeGenTypes::arrangeLLVMFunctionInfo() calls SystemZABIInfo::computeInfo() and the argument types are classified. How about Swift (or SPIR_KERNEL) CCs: should they also have narrow integer arguments extended on SystemZ?)

- IR: new attribute NoExt, handled by parser, emitter etc. Adding one bit IsNoExt ArgFlagsTy grew it past the 3-byte size, so the static_assert was incremented to match the new size of 4 bytes. Any problem with this?

- SystemZ backend:
  - Default sign extension of <=32 bit arguments added, which can be disabled with '-no-arg-exts'.
  - New option -verify-int-arg-exts (default off), that when passed checks that all <=32 bit integer arguments have a valid extension attribute. Verifying extension of return values and outgoing call arguments: Incoming function parameters and call results could also be checked, but they are not needed for correctness, but only to let the backend assume they are sign-extended properly.
  - A lot of tests updated, mostly with -no-arg-exts, which makes sense in cases like testing for a three-address instruction selection. It seems reasonable to simply disable this default sign-extension during testing, but in a way it may in the future be better to instead update the tests. There are more tests that can be updated - I have only changed the ones that seemed trivial so far. (args-04.ll: This is testing incoming arguments, but it is returning them without extension. This is a little weird as structs are never returned in a reg..?)

- Documentation: Explanation of the NoExt attribute and why it is mandatory.

The comment for getTypeForExtReturn() may be related to this issue..? "// Return the type that should be used to zero or sign extend a zeroext/signext integer return value.  FIXME: Some C calling conventions require the return type to be promoted, but this is not true all the time, e.g. i1/i8/i16 on x86/x86_64. It is also not necessary for non-C calling conventions. The frontend should handle this and include all of the necessary information."

So far, this is a work in progress so feel free to make suggestions...

https://reviews.llvm.org/D112942

Files:
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/SystemZ/systemz-abi-vector.c
  clang/test/CodeGen/SystemZ/systemz-abi.c
  clang/test/CodeGen/SystemZ/systemz-abi.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/CodeGen/SystemZ/and-02.ll
  llvm/test/CodeGen/SystemZ/and-07.ll
  llvm/test/CodeGen/SystemZ/args-01.ll
  llvm/test/CodeGen/SystemZ/args-04.ll
  llvm/test/CodeGen/SystemZ/args-11.ll
  llvm/test/CodeGen/SystemZ/args-12.ll
  llvm/test/CodeGen/SystemZ/args-13.ll
  llvm/test/CodeGen/SystemZ/args-14.ll
  llvm/test/CodeGen/SystemZ/args-15.ll
  llvm/test/CodeGen/SystemZ/args-16.ll
  llvm/test/CodeGen/SystemZ/args-17.ll
  llvm/test/CodeGen/SystemZ/args-18.ll
  llvm/test/CodeGen/SystemZ/args-19.ll
  llvm/test/CodeGen/SystemZ/asm-11.ll
  llvm/test/CodeGen/SystemZ/asm-12.ll
  llvm/test/CodeGen/SystemZ/asm-13.ll
  llvm/test/CodeGen/SystemZ/asm-14.ll
  llvm/test/CodeGen/SystemZ/asm-15.ll
  llvm/test/CodeGen/SystemZ/asm-16.ll
  llvm/test/CodeGen/SystemZ/asm-17.ll
  llvm/test/CodeGen/SystemZ/asm-18.ll
  llvm/test/CodeGen/SystemZ/atomic-load-01.ll
  llvm/test/CodeGen/SystemZ/atomic-load-02.ll
  llvm/test/CodeGen/SystemZ/atomic-load-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-add-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-add-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-add-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-add-05.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-and-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-and-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-and-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-and-05.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-minmax-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-minmax-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-minmax-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-nand-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-nand-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-nand-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-or-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-or-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-or-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-or-05.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-sub-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-sub-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-sub-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-sub-05.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xchg-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xchg-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xchg-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xor-01.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xor-02.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xor-03.ll
  llvm/test/CodeGen/SystemZ/atomicrmw-xor-05.ll
  llvm/test/CodeGen/SystemZ/branch-05.ll
  llvm/test/CodeGen/SystemZ/branch-11.ll
  llvm/test/CodeGen/SystemZ/bswap-02.ll
  llvm/test/CodeGen/SystemZ/bswap-06.ll
  llvm/test/CodeGen/SystemZ/call-03.ll
  llvm/test/CodeGen/SystemZ/cmpxchg-01.ll
  llvm/test/CodeGen/SystemZ/cmpxchg-02.ll
  llvm/test/CodeGen/SystemZ/cmpxchg-03.ll
  llvm/test/CodeGen/SystemZ/cmpxchg-04.ll
  llvm/test/CodeGen/SystemZ/cmpxchg-06.ll
  llvm/test/CodeGen/SystemZ/codegenprepare-form-OF-ops.ll
  llvm/test/CodeGen/SystemZ/cond-move-02.ll
  llvm/test/CodeGen/SystemZ/cond-move-06.ll
  llvm/test/CodeGen/SystemZ/ctpop-01.ll
  llvm/test/CodeGen/SystemZ/fp-conv-09.ll
  llvm/test/CodeGen/SystemZ/fp-conv-10.ll
  llvm/test/CodeGen/SystemZ/fp-conv-14.ll
  llvm/test/CodeGen/SystemZ/fp-conv-16.ll
  llvm/test/CodeGen/SystemZ/fp-move-02.ll
  llvm/test/CodeGen/SystemZ/fp-strict-conv-09.ll
  llvm/test/CodeGen/SystemZ/fp-strict-conv-10.ll
  llvm/test/CodeGen/SystemZ/fp-strict-conv-14.ll
  llvm/test/CodeGen/SystemZ/fp-strict-conv-16.ll
  llvm/test/CodeGen/SystemZ/fpc-intrinsics.ll
  llvm/test/CodeGen/SystemZ/htm-intrinsics.ll
  llvm/test/CodeGen/SystemZ/inline-asm-i128.ll
  llvm/test/CodeGen/SystemZ/insert-01.ll
  llvm/test/CodeGen/SystemZ/int-abs-01.ll
  llvm/test/CodeGen/SystemZ/int-add-13.ll
  llvm/test/CodeGen/SystemZ/int-add-14.ll
  llvm/test/CodeGen/SystemZ/int-cmp-36.ll
  llvm/test/CodeGen/SystemZ/int-cmp-38.ll
  llvm/test/CodeGen/SystemZ/int-cmp-44.ll
  llvm/test/CodeGen/SystemZ/int-cmp-45.ll
  llvm/test/CodeGen/SystemZ/int-cmp-57.ll
  llvm/test/CodeGen/SystemZ/int-cmp-61.ll
  llvm/test/CodeGen/SystemZ/int-const-01.ll
  llvm/test/CodeGen/SystemZ/int-conv-01.ll
  llvm/test/CodeGen/SystemZ/int-conv-02.ll
  llvm/test/CodeGen/SystemZ/int-conv-05.ll
  llvm/test/CodeGen/SystemZ/int-conv-06.ll
  llvm/test/CodeGen/SystemZ/int-conv-13.ll
  llvm/test/CodeGen/SystemZ/int-div-01.ll
  llvm/test/CodeGen/SystemZ/int-div-06.ll
  llvm/test/CodeGen/SystemZ/int-move-01.ll
  llvm/test/CodeGen/SystemZ/int-move-02.ll
  llvm/test/CodeGen/SystemZ/int-move-08.ll
  llvm/test/CodeGen/SystemZ/int-move-10.ll
  llvm/test/CodeGen/SystemZ/int-mul-05.ll
  llvm/test/CodeGen/SystemZ/int-mul-11.ll
  llvm/test/CodeGen/SystemZ/int-neg-01.ll
  llvm/test/CodeGen/SystemZ/int-neg-02.ll
  llvm/test/CodeGen/SystemZ/int-sub-08.ll
  llvm/test/CodeGen/SystemZ/int-uadd-06.ll
  llvm/test/CodeGen/SystemZ/int-usub-06.ll
  llvm/test/CodeGen/SystemZ/knownbits-intrinsics-binop.ll
  llvm/test/CodeGen/SystemZ/knownbits.ll
  llvm/test/CodeGen/SystemZ/memcmp-01.ll
  llvm/test/CodeGen/SystemZ/not-01.ll
  llvm/test/CodeGen/SystemZ/or-07.ll
  llvm/test/CodeGen/SystemZ/pr42512.ll
  llvm/test/CodeGen/SystemZ/risbg-01.ll
  llvm/test/CodeGen/SystemZ/risbg-02.ll
  llvm/test/CodeGen/SystemZ/risbg-04.ll
  llvm/test/CodeGen/SystemZ/rot-01.ll
  llvm/test/CodeGen/SystemZ/rot-02.ll
  llvm/test/CodeGen/SystemZ/scalar-ctlz.ll
  llvm/test/CodeGen/SystemZ/selectcc-01.ll
  llvm/test/CodeGen/SystemZ/selectcc-02.ll
  llvm/test/CodeGen/SystemZ/setcc-01.ll
  llvm/test/CodeGen/SystemZ/setcc-02.ll
  llvm/test/CodeGen/SystemZ/sext-zext.ll
  llvm/test/CodeGen/SystemZ/shift-01.ll
  llvm/test/CodeGen/SystemZ/shift-02.ll
  llvm/test/CodeGen/SystemZ/shift-03.ll
  llvm/test/CodeGen/SystemZ/shift-04.ll
  llvm/test/CodeGen/SystemZ/shift-09.ll
  llvm/test/CodeGen/SystemZ/shift-11.ll
  llvm/test/CodeGen/SystemZ/shift-12.ll
  llvm/test/CodeGen/SystemZ/soft-float-args.ll
  llvm/test/CodeGen/SystemZ/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/SystemZ/stack-clash-protection.ll
  llvm/test/CodeGen/SystemZ/store_nonbytesized_vecs.ll
  llvm/test/CodeGen/SystemZ/strcmp-01.ll
  llvm/test/CodeGen/SystemZ/strcpy-01.ll
  llvm/test/CodeGen/SystemZ/strcpy-nobuiltin.ll
  llvm/test/CodeGen/SystemZ/swift-return.ll
  llvm/test/CodeGen/SystemZ/swifterror.ll
  llvm/test/CodeGen/SystemZ/tdc-01.ll
  llvm/test/CodeGen/SystemZ/tdc-06.ll
  llvm/test/CodeGen/SystemZ/tdc-07.ll
  llvm/test/CodeGen/SystemZ/trap-01.ll
  llvm/test/CodeGen/SystemZ/trap-02.ll
  llvm/test/CodeGen/SystemZ/trap-03.ll
  llvm/test/CodeGen/SystemZ/unaligned-01.ll
  llvm/test/CodeGen/SystemZ/vec-combine-02.ll
  llvm/test/CodeGen/SystemZ/vec-extract-02.ll
  llvm/test/CodeGen/SystemZ/vec-intrinsics-01.ll
  llvm/test/CodeGen/SystemZ/vec-intrinsics-02.ll
  llvm/test/CodeGen/SystemZ/vec-move-05.ll
  llvm/test/CodeGen/SystemZ/xor-07.ll
  llvm/test/DebugInfo/Generic/missing-abstract-variable.ll
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/systemz_function_name.ll
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/systemz_function_name.ll.expected
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/systemz_generated_funcs.ll
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/systemz_generated_funcs.ll.generated.expected
  llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/systemz_generated_funcs.ll.nogenerated.expected

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112942.383839.patch
Type: text/x-patch
Size: 128113 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211101/4546ecd7/attachment-0001.bin>