[PATCH] D139209: [IRMover] Remove UB implying parameter attributes when necessary

[Tim Neumann via Phabricator via llvm-commits]

TimNN added a comment.

The current code is failing for a situation like this:

  --- SRC ---
  ; Module Name: lib/libLLVMCodeGen.a(TargetLoweringBase.cpp.o at 275472348)
  ; Module Source: /home/logic/Projects/llvm/llvm/lib/CodeGen/TargetLoweringBase.cpp
  ; Materializable
  ; Function Attrs: mustprogress noinline nounwind optnone uwtable
  define { ptr, i8 } @_ZNK4llvm18TargetLoweringBase23findRepresentativeClassEPKNS_18TargetRegisterInfoENS_3MVTE(ptr noundef nonnull align 8 dereferenceable(218195) %0, ptr noundef %1, i8 %2) unnamed_addr #1 align 2 {}
  
  --- DST ---
  ; Module Name: lib/libLLVMAArch64CodeGen.a(AArch64ISelLowering.cpp.o at 48495784)
  ; Module Source: /home/logic/Projects/llvm/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  declare void @_ZNK4llvm18TargetLoweringBase23findRepresentativeClassEPKNS_18TargetRegisterInfoENS_3MVTE() unnamed_addr

It seems like when generating vtables, the code at https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGVTables.cpp may generate function declarations with an "empty/void" signature for the `IsUnprototyped` case.

Since clang is deliberately generating this code, I'm assuming this is considered valid according to LLVM IR semantics. I'm not sure what the most appropriate fix here would be. I primarily see three open questions:

**Question 1:** Is my assumption correct and this is valid LLVM IR?

**Question 2:** Should we attempt to detect this case specifically (DST has zero args and returns void) but otherwise still assert that the argument lists have the same length? Or should we handle all cases of argument lists with different lengths?

**Question 3:** For the cases detected by (1), should we conservatively remove UB-implying attributes from all arguments (erring on the side of correctness)? Or should we just bail out and not do anything (erring on the side of performance)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139209/new/

https://reviews.llvm.org/D139209