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 Oct 30 2022, 2:09 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

Depends on D137049

Diff Detail

Event Timeline

vzakhari created this revision.Oct 30 2022, 2:09 PM
Herald added a project: Restricted Project. · View Herald Transcript
vzakhari requested review of this revision.Oct 30 2022, 2:09 PM
clementval accepted this revision.Oct 31 2022, 1:32 AM

LGTM

flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
449

Don't we have a getFirRuntimeAttrName or smth like that instead of hardcoding the value here?

This revision is now accepted and ready to land.Oct 31 2022, 1:32 AM
vzakhari added inline comments.Oct 31 2022, 9:21 AM
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
449

I will add it. Thanks!

vzakhari updated this revision to Diff 472064.Oct 31 2022, 10:48 AM

Added fir::FIROpsDialect::getFirRuntimeAttrName().

Just an observation, it turns out we are currently not generating anything from flang/include/flang/Optimizer/Dialect/FIRDialect.td

vzakhari updated this revision to Diff 472840.Nov 2 2022, 8:44 PM

Reworked the patch to set the attributes on the result not on the operation itself.

vzakhari requested review of this revision.Nov 2 2022, 8:44 PM
clementval accepted this revision.Nov 3 2022, 1:40 AM

LGTM

flang/include/flang/Optimizer/Dialect/FIRDialect.h
41 ↗(On Diff #472840)

We have a couple of attribute name also in flang/include/flang/Optimizer/Dialect/FIROpsSupport.h. Might be a good thing to consolidate this sometime.

This revision is now accepted and ready to land.Nov 3 2022, 1:40 AM
vzakhari added inline comments.Nov 3 2022, 8:43 AM
flang/include/flang/Optimizer/Dialect/FIRDialect.h
41 ↗(On Diff #472840)

Thank you, Valentin! I will add constexpr before the merge.

I think we'd better define the attributes the same way as in https://reviews.llvm.org/D135961#change-qUcQEfaF2qpe and stop using strings.

Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2022, 12:53 PM

I suspect this might be causing arm windows buildbot failure: https://lab.llvm.org/buildbot/#/builders/65/builds/7283

I was not able to reproduce it on x86 windows, so I am going to revert it and investigate further.