[PATCH] D28166: Properly merge K&R functions that have attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D28166#633643, @rsmith wrote:

> In https://reviews.llvm.org/D28166#633621, @aaron.ballman wrote:
>
> > Do you think this patch should be gated on (or perhaps combined with) a fix for the lowering bug, or do you think this patch is reasonable on its own? Given that it turns working code into UB, I think my preference is to gate it on a fix for the lowering bug, but I'm also not certain I am the right person to implement that fix (though I could give it a shot).
>
>
> The test in question has a comment pointing to PR7117, which in turn indicates that we might miscompile parts of FreeBSD if we land this as-is. So I think we need to gate this on a fix for the lowering bug.


I agree with that logic, but would also point out that this bug appears to have nothing to do with attributed functions, but K&R calls in general. Removing the `__stdcall` entirely shows an identical issue:

  void g(int n) {}
  void (*p)() = g;
  void f() { p(0);}

generates

  ; ModuleID = '-'
  source_filename = "-"
  target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
  target triple = "x86_64-pc-windows-msvc"
  
  @p = global void (...)* bitcast (void (i32)* @g to void (...)*), align 8
  
  ; Function Attrs: noinline nounwind
  define void @g(i32 %n) #0 {
  entry:
    %n.addr = alloca i32, align 4
    store i32 %n, i32* %n.addr, align 4
    ret void
  }
  
  ; Function Attrs: noinline nounwind
  define void @f() #0 {
  entry:
    %0 = load void (...)*, void (...)** @p, align 8
    %callee.knr.cast = bitcast void (...)* %0 to void (i64)*
    call void %callee.knr.cast(i64 0)
    ret void
  }
  
  attributes #0 = { noinline nounwind "correctly-rounded-divide-sqrt-fp-math"="fal
  se" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-
  elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-mat
  h"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-p
  rotector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-ma
  th"="false" "use-soft-float"="false" }
  
  !llvm.ident = !{!0}
  
  !0 = !{!"clang version 4.0.0 (trunk 290670)"}

I'm happy to try to work on fixing this, but am unfamiliar with this part of the codegen. I *think* the problem is that we gin up the function type for a non-prototyped function based on the function call expression argument types, and the literal `0` is getting the type `signed long long`. This forces us to bitcast the function to one taking an i64 rather than an i32. If you switch the argument from the literal `0` to a variable of type `int`, the bitcast becomes `%3 = bitcast void (...)* %1 to void (i32)*`, which is correct (at least, as I interpret the comments left in `EmitCall()` around line 4245).

Suggestions and hints welcome. :-)


https://reviews.llvm.org/D28166