llvm-gcc doesn't handle struct-returning stdcall functions right

[asl]

Bugzilla Link	1131
Resolution	FIXED
Resolved on	Feb 22, 2010 12:44
Version	trunk
OS	All
Attachments	Failed bytecode, Preprocessed source, Just one insteresting function extracted, Updated bytecode, The source of prev. bytecode

Extended Description

Consider the attached bytecode.

./opt -instcombine -simplifycfg nsSubstring.bc

strips all code from %_ZN11nsSubstring7ReplaceEjjRK18nsAString_internal function
(the same for "EjjRK19" one). Without instcombine - everything is ok.

This breaks Mozilla builds.

[asl]

assigned to @asl

[llvmbot]

The problem seems to be in instcombine, which is turning:

%tmp = alloca %struct.nsAStringAccessor, align 16 ; <%struct.nsAStringAccessor*> [#uses=3]
"alloca point" = bitcast i32 0 to i32 ; [#uses=0]
call csretcc void %_ZNK18nsAString_internal11ToSubstringEv( %struct.nsAStringAccessor* %tmp, %
struct.nsAString_internal* %readable )

into:

%tmp = alloca %struct.nsAStringAccessor, align 16 ; <%struct.nsAStringAccessor*> [#uses=2]
store i1 true, i1* undef

simplifycfg is then just simplifying the undefined behavior into an unreachable. The question is why is
instcombine doing this.

Also worth noting: the whole problem disappears if you remove csretcc from the call to %
_ZNK18nsAString_internal11ToSubstringEv

[lattner]

Excellent catch Owen. The reason instcombine is doing this is because of undefined behavior in the input
code. In particular, you have a call to '_ZNK18nsAString_internal11ToSubstringEv'. The call uses the
calling convention 'csretcc' and the function it calls has no CC defined.

Two possibilities:

1. This is a bug in llvm-extract, perhaps it's not retaining the CC when code is extracted.
2. This is a bug in the front-end w.r.t calling conventions.

-Chris

[lattner]

Okay, it looks like both :). _ZNK18nsAString_internal11ToSubstringEv is defined with x86_stdcallcc and
called with csretcc, so that's a f.e. bug. llvm-extract is also losing the CC information. I'll fix llvm-extract.

-Chris

[asl]

It was decided to drop csret CC at all and introduce new parameter attribute
"sret", which will mark first argument used as "hidden pointer to result". I'll
perform the fixes:

1. Remove csretcc from everything
2. Add helper method to quickly check, whether function returns struct with
   hidden argument
3. Add corresponding calls & function arguments lowering
4. Update verifier to check, that this attribute should be set only on first
   argument.

[lattner]

verifier should also check that the argument is a pointer. Otherwise, sounds right!

[asl]

Fixed in:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070122/043385.html
....
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070122/043401.html

Will be closed as soon as llvm-upgrade will be ready.

[lattner]

Reid recently committed the llvm-upgrade pieces, this is done. Thanks guys!