This is an archive of the discontinued LLVM Phabricator instance.

[flang] Use proper attributes for runtime calls with 'i1' arguments/returns.
ClosedPublic

Authored by vzakhari on Jan 26 2023, 4:51 PM.

Details

Summary

Clang uses signext/zeroext attributes for integer arguments shorter than
the default 'int' type on a target. So Flang has to match this for functions
from Fortran runtime and also for BIND(C) routines. This patch implements
ABI adjustments only for Fortran runtime calls. BIND(C) part will be done
separately.

This resolves https://github.com/llvm/llvm-project/issues/58579

Diff Detail

Event Timeline

vzakhari created this revision.Jan 26 2023, 4:51 PM
vzakhari requested review of this revision.Jan 26 2023, 4:51 PM
vzakhari planned changes to this revision.Jan 26 2023, 4:51 PM
vzakhari added inline comments.Jan 27 2023, 6:11 PM
flang/lib/Optimizer/CodeGen/Target.cpp
216

I've been chasing segfaults caused by this commit in Windows ARM buildbot (e.g. https://lab.llvm.org/buildbot/#/builders/65/builds/7378).

It seems that the piece of code below is not working correctly:

mlir::TypeRange range = {eleTy, eleTy};
mlir::Type tupleTy = mlir::TupleType::get(eleTy.getContext(), range);
eleTy.dump();
tupleTy.dump();
mlir::Type newTy = mlir::TupleType::get(eleTy.getContext(), range);
newTy.dump();
marshal.emplace_back(newTy, AT{});

The printout is:

!fir.real<8>
tuple<!fir.real<8>, !fir.real<8>>
tuple<tuple<!fir.real<8>, !fir.real<8>>, !fir.real<8>>

Note the 3rd dump: it means the newTy was created as if the range held tupleTy and eleTy. So range is obviously pointing to the stack in a wrong way. Without my modifications we end up creating a TupleType with junk component types - this breaks everything after that.

I do not think the lifetime extension for the temporary underlying array of the initializer_list applies in this case, so all these range uses in the file seem to be invalid. I am not sure why my changes trigger the error. My only guess is that adding more virtual methods into the base class somehow affected optimizations.

I can get clean check-flang after fixing all instances of such TypeRange usage, so this is what I am going to do to finally make progress with this commit.

vzakhari updated this revision to Diff 492967.Jan 27 2023, 6:53 PM
vzakhari retitled this revision from [WIP][flang] Use proper attributes for runtime calls with 'i1' arguments/returns. to [flang] Use proper attributes for runtime calls with 'i1' arguments/returns..
vzakhari edited the summary of this revision. (Show Details)
vzakhari added reviewers: clementval, jeanPerier.
  • Fixed TypeRange usage.
clementval added inline comments.Jan 30 2023, 7:18 AM
flang/lib/Lower/IntrinsicCall.cpp
38 ↗(On Diff #492967)

Do you still need it?

vzakhari added inline comments.Jan 30 2023, 9:08 AM
flang/lib/Lower/IntrinsicCall.cpp
38 ↗(On Diff #492967)

It is not needed. Thanks!

vzakhari updated this revision to Diff 493332.Jan 30 2023, 9:09 AM
  • Removed redundant include.

Looks like there is a failure on Driver/target-cpu-features.f90. Not sure it is related.

Looks like there is a failure on Driver/target-cpu-features.f90. Not sure it is related.

I think it is due to D142548. I will fix it separately.

@dmgreen, FYI

This revision is now accepted and ready to land.Jan 30 2023, 12:35 PM