le32-unknown-nacl target does not optimize simple struct return values as primitive returns

[Quuxplusone]

Bugzilla Link	PR18863
Status	REOPENED
Importance	P normal
Reported by	Chad Austin (caustin@gmail.com)
Reported on	2014-02-16 14:14:11 -0800
Last modified on	2014-02-26 10:20:31 -0800
Version	3.4
Hardware	PC All
CC	alonzakai@gmail.com, dschuff@google.com, jvoung@google.com, llvm-bugs@lists.llvm.org, llvm-bugzilla@jfbastien.com, llvm@sunfishcode.online, rafael@espindo.la
Fixed by commit(s)
Attachments	sret.cpp (160 bytes, text/plain) sret-amd64.ll (1414 bytes, application/octet-stream) sret-le32.ll (1533 bytes, application/octet-stream)
Blocks	PR15465
Blocked by
See also

Created attachment 12072
sret.cpp

The attached sret.cpp demonstrates a simple struct with a nondefault
constructor.

When compiled to IR for the default x86_64 target on Ubuntu 12.04 AMD64, it
optimizes the S struct into a simple i32.  See the attached sret-amd64.ll.

When compiled to IR for the le32-unknown-nacl target, the struct is not
optimized away.  See the attached sret-le32.ll.

Both files were generated with:

clang --std=c++11 -O2 -emit-llvm -target le32-unknown-nacl -c -Wall -o sret.bc
sret.cpp && llvm-dis -o sret-le32.ll sret.bc

clang --std=c++11 -O2 -emit-llvm -c -Wall -o sret.bc sret.cpp && llvm-dis -o
sret-amd64.ll sret.bc

This reproduces with both clang 3.3 and clang 3.4.

The lack of this optimization penalizes use of utility types, such as
InternedString and IntrusiveSmartPtr, used frequently in our Emscripten-
compiled project.

[Quuxplusone]

Attached sret.cpp (160 bytes, text/plain): sret.cpp

[Quuxplusone]

Attached sret-amd64.ll (1414 bytes, application/octet-stream): sret-amd64.ll

[Quuxplusone]

Attached sret-le32.ll (1533 bytes, application/octet-stream): sret-le32.ll

[Quuxplusone]

Changing how clang lowers this would break the NaCl ABI, and I don't know what we can do there. It should be pretty easy to fix in the emscripten back end though.

[Quuxplusone]

Ok, then I guess we should fix this in the new emscripten backend (which will then hopefully get upstreamed eventually when the backend is ready together with everything else).

I think we can close this and focus on

https://github.com/kripken/emscripten/issues/2135

[Quuxplusone]

I think this is part of the le32-*-* behavior, and talking to jvoung/dschuff it
sounds like we initially wanted to allow struct returns in PNaCl but then
removed struct types to simplify things, so in PNaCl structs with a single
integer get transformed to a single integers in a later pass. In NaCl there is
no ABI issue (the IRT is the only external consumer of the ABI adn we build it
ourselves). Same stands for Emscripten.

It therefore seems like changing Clang to emit int returns when returning a
struct with a single integer in le32-*-* mode is acceptable.

[Quuxplusone]

As a reference, Dan's asmjs-unknown-emscripten patch:
  https://github.com/kripken/emscripten-fastcomp-clang/commit/f6e2fddff6294c1c2ec2f9602c30ff680359ed19