AllocOpt: Fix stack lowering where alloca continas boxed and unboxed data

[wsmoses]

…-julia objects

cc @gbaraldi

[wsmoses]

Fixes #55305

[vchuravy]

The comments are confusing. The correct terminology is boxed and unboxed not Julia and not-Julia.

Is there an end to end test case?

[vchuravy]

This also feels like the wrong direction. An Julia object that just contains unboxed fields should be easy to move to the stack. Julia objects that contain pointers on boxed objects need to be carefully handled since we need to potentially root the pointers stored in the fields.

[wsmoses]

Best end-to-end case we got presently is: EnzymeAD/Enzyme.jl#1652 (comment)

The problem atm is essentially that { i8*, {} addr(10) } cannot be upgraded to a stack alloca. It presently is and causes a segfault, in particular see #55305 for an explanation and demonstration of the segfault caused by the present behavior

[vchuravy]

But this not an issue in alloc opt. This is an issue with the GC rooting pass? Moving this malloc to an alloca should be legal, but the rooting pass needs to handle it.

[wsmoses]

Alloca opt doesn't turn a gc alloc of a { i64, {} addr(10)* } to an alloca of { i64, {} addr(10)* }. It turns it into [2 x addr(10)] to be bitcast to { i64, {} addr(10) } . Thus rooting is correct, if the alloca [2 x addr(10)*] can be believed.

[wsmoses]

@vchuravy @gbaraldi the failures look like an oom on the machine itself and unrelated?

[vchuravy]

Please update the comments to refer the correct terminology. There is no such thing as a "non-julia-object" in this context.

I am still unclear if this is just an issue with access to unboxed, boxed fields with unkown indices? Since IIUC the issue is that we make the assumption for use_ref that all fields are references.

if (has_bits && has_ref) { needs a comment explaining this situation in detail.

[wsmoses]

@vchuravy done

[vchuravy]

Doesn't need backport to 1.6 since #47076 was never backported.

[vchuravy]

    ANALYZE src/clang-tidy-llvm-alloc-opt
Error while processing /cache/build/tester-amdci5-15/julialang/julia-master/src/llvm-alloc-opt.cpp.
/cache/build/tester-amdci5-15/julialang/julia-master/src/llvm-alloc-opt.cpp:255:37: error: no member named 'has_unknown_bits' in 'jl_alloc::AllocUseInfo'; did you mean 'has_unknown_objref'? [clang-diagnostic-error]
  255 |         bool has_unboxed = use_info.has_unknown_bits;
      |                                     ^~~~~~~~~~~~~~~~
      |                                     has_unknown_objref
/cache/build/tester-amdci5-15/julialang/julia-master/src/llvm-alloc-helpers.h:97:14: note: 'has_unknown_objref' declared here
   97 |         bool has_unknown_objref:1;
      |              ^
/cache/build/tester-amdci5-15/julialang/julia-master/src/llvm-alloc-opt.cpp:260:34: error: no member named 'hasbits' in 'jl_alloc::Field' [clang-diagnostic-error]
  260 |             has_unboxed |= field.hasbits;
      |                            ~~~~~ ^
make: *** [Makefile:535: clang-tidy-llvm-alloc-opt] Error 1

[wsmoses]

@vchuravy this all seems to pass, if it looks good to merge?

[KristofferC]

There is some merge conflict in the alloc opt LLVM test here when backporting to 1.10.

[wsmoses]

@gbaraldi I didn't realize this wasn't successfully backported to 1.10, would you be able to take a look?

[wsmoses]

@KristofferC do you know offhand if this was early enough to be included in 1.11, or if it needs backport?

[wsmoses]

ah nevermind I see the relevant commit above.