vsapsai added inline comments.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2321
+ !isa<llvm::StructType>(ConvertType(Arg->getType())) &&
ArgI.getCoerceToType() == ConvertType(Ty) &&
ArgI.getDirectOffset() == 0) {
----------------
rjmccall wrote:
> I think the right fix is to change the second clause to
> ArgI.getCoerceToType() == ConvertType(Arg->getType()). The point of this
> special case is to catch the common case that the natural way that IRGen
> wants to emit the argument expression is by producing a single scalar of
> exactly the right IR type; it seems to me that that condition would capture
> that correctly.
I've tried that and it almost works. There are 4 failing tests
Clang :: CodeGen/kr-func-promote.c
Clang :: CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
Clang :: CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
Clang :: CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
In a couple of places the problem is that expected function `i32 @a(i32)` but
received `i32 @a(i32 %x.coerce)`.
As for me, parameter naming is not really a problem and can be fixed by making
tests less specific. What is more interesting, with the change we started
adding extra store and load. For example, for CodeGen/kr-func-promote.c IR was
```
define i32 @a(i32) #0 {
%x.addr = alloca i16, align 2
%x = trunc i32 %0 to i16
store i16 %x, i16* %x.addr, align 2
%2 = load i16, i16* %x.addr, align 2
%conv = sext i16 %2 to i32
ret i32 %conv
}
```
and with type comparison change IR becomes
```
define i32 @a(i32 %x.coerce) #0 {
entry:
%x = alloca i16, align 2
%x.addr = alloca i16, align 2
%coerce.val.ii = trunc i32 %x.coerce to i16
store i16 %coerce.val.ii, i16* %x, align 2
%x1 = load i16, i16* %x, align 2
store i16 %x1, i16* %x.addr, align 2
%0 = load i16, i16* %x.addr, align 2
%conv = sext i16 %0 to i32
ret i32 %conv
}
```
The same situation is with "microsoft-abi-*" tests where instead of
```
%this = bitcast i8* %0 to %struct.D*
store %struct.D* %this, %struct.D** %this.addr, align 4
%this1 = load %struct.D*, %struct.D** %this.addr, align 4
%2 = bitcast %struct.D* %this1 to i8*
```
we have
```
%coerce.val = bitcast i8* %this.coerce to %struct.D*
store %struct.D* %coerce.val, %struct.D** %this, align 4
%this1 = load %struct.D*, %struct.D** %this, align 4
store %struct.D* %this1, %struct.D** %this.addr, align 4
%this2 = load %struct.D*, %struct.D** %this.addr, align 4
%0 = bitcast %struct.D* %this2 to i8*
```
I'll check where and why we are adding extra store/load.
https://reviews.llvm.org/D41311
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
