[all-commits] [llvm/llvm-project] e640d9: [RISCV][GlobalISel] Fix legalizing ‘llvm.va_copy’ ...

[bvlgah via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e640d9e725ef06b9787ab0ab884598f4f5532e48
      https://github.com/llvm/llvm-project/commit/e640d9e725ef06b9787ab0ab884598f4f5532e48
  Author: bvlgah <octopus.busts_0w at icloud.com>
  Date:   2024-03-28 (Thu, 28 Mar 2024)

  Changed paths:
    M llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp
    M llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-vacopy.mir
    M llvm/test/CodeGen/RISCV/GlobalISel/vararg.ll

  Log Message:
  -----------
  [RISCV][GlobalISel] Fix legalizing ‘llvm.va_copy’ intrinsic (#86863)

Hi, I spotted a problem when running benchmarking programs on a RISCV64
device.

## Issue

Segmentation faults only occurred while running the programs compiled
with `GlobalISel` enabled.

Here is a small but complete example (it is adopted from [Google's
benchmark
framework](https://github.com/llvm/llvm-test-suite/blob/95a9f0d0b45056274f0bb4b0e0dd019023e414dc/MicroBenchmarks/libs/benchmark/src/colorprint.cc#L85-L119)
to reproduce the issue,

```cpp
#include <cstdarg>
#include <cstdio>
#include <iostream>
#include <memory>
#include <string>

std::string FormatString(const char* msg, va_list args) {
  // we might need a second shot at this, so pre-emptivly make a copy
  va_list args_cp;
  va_copy(args_cp, args);

  std::size_t size = 256;
  char local_buff[256];
  auto ret = vsnprintf(local_buff, size, msg, args_cp);

  va_end(args_cp);

  // currently there is no error handling for failure, so this is hack.
  // BM_CHECK(ret >= 0);

  if (ret == 0)  // handle empty expansion
    return {};
  else if (static_cast<size_t>(ret) < size)
    return local_buff;
  else {
    // we did not provide a long enough buffer on our first attempt.
    size = static_cast<size_t>(ret) + 1;  // + 1 for the null byte
    std::unique_ptr<char[]> buff(new char[size]);
    ret = vsnprintf(buff.get(), size, msg, args);
    // BM_CHECK(ret > 0 && (static_cast<size_t>(ret)) < size);
    return buff.get();
  }
}

std::string FormatString(const char* msg, ...) {
  va_list args;
  va_start(args, msg);
  auto tmp = FormatString(msg, args);
  va_end(args);
  return tmp;
}

int main() {
  std::string Str =
      FormatString("%-*s %13s %15s %12s", static_cast<int>(20),
                   "Benchmark", "Time", "CPU", "Iterations");
  std::cout << Str << std::endl;
}
```

Use `clang++ -fglobal-isel -o main main.cpp` to compile it.

## Cause

I have examined MIR, it shows that these segmentation faults resulted
from a small mistake about legalizing the intrinsic function
`llvm.va_copy`.


https://github.com/llvm/llvm-project/blob/36e74cfdbde208e384c72bcb52ea638303fb7d67/llvm/lib/Target/RISCV/GISel/RISCVLegalizerInfo.cpp#L451-L453

`DstLst` and `Tmp` are placed in the wrong order.

## Changes

I have tweaked the test case `CodeGen/RISCV/GlobalISel/vararg.ll` so
that `s0` is used as the frame pointer (not in all checks) which points
to the starting address of the save area. I believe that it helps reason
about how `llvm.va_copy` is handled.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications