[all-commits] [llvm/llvm-project] dca40e: [CodeGen] Replace CCValAssign::Loc with std::varia...

Sergei Barannikov via All-commits all-commits at lists.llvm.org
Sun Jan 15 00:07:42 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: dca40e3288f481cda1ce37c647b782a3b468b7d1
      https://github.com/llvm/llvm-project/commit/dca40e3288f481cda1ce37c647b782a3b468b7d1
  Author: Sergei Barannikov <barannikov88 at gmail.com>
  Date:   2023-01-15 (Sun, 15 Jan 2023)

  Changed paths:
    M llvm/include/llvm/CodeGen/CallingConvLower.h
    M llvm/lib/CodeGen/CallingConvLower.cpp
    M llvm/lib/Target/Sparc/SparcISelLowering.cpp

  Log Message:
  -----------
  [CodeGen] Replace CCValAssign::Loc with std::variant (NFCI)

The motivation behind this change is as follows.
Targets with stack growing up (there are no such in-tree targets) pass
arguments at negative offsets relative to the stack pointer. This makes
it hard to use the generic value assigner because CCValAssign stores the
offset as an unsigned integer, which is then zero-extended when
converted to int64_t, e.g. when passing to `CreateFixedObject`. This
results in conversion of, for example, -4 into 4294967292, which is not
desired.

While it is possible to insert a cast to `int` before passing the result
of `getLocMemOffset` into `CreateFixedObject` in backend code, this is
error-prone, and some uses of `getLocMemOffset` are located in
places common to all backends (e.g. `CallLowering::handleAssignments`).

That said, I wanted to change the type of the memory offset from
`unsigned` to `int64_t` (this would be consistent with other places
where stack offsets are used). However, the `Loc` field which stores the
offset is shared between three different kinds of the location:
register, memory, and "pending". Storing a register number as `int64_t`
does not seem right (there are `Register` and `MCRegister` for this), so
I did the most straightforward change - replaced the `Loc` field with
std::variant.

The main change that changes the type of the memory offset from
`unsigned` to `int64_t` will be in a follow-up patch to simplify the
review.

Reviewed By: MaskRay, nikic

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




More information about the All-commits mailing list