This patch fixes lowering of elemental character MIN/MAX in HLFIR.
Previously, these cases could hit a lowering-time TODO
`ElementalIntrinsicCallBuilder::computeDynamicCharacterResultLength` and
abort. This change computes the character result length as the maximum
length of the present actual arguments, allowing valid elemental
character MIN/MAX calls to lower successfully.
Added regression coverage for elemental character MIN/MAX, including
differing-length arguments.
Co-authored-by: Sairudra More <moresair@pe31.hpc.amslabs.hpecorp.net>
When ignore_tkr(c) is set and the actual argument is an allocatable or
pointer (stored as a descriptor), the lowering code was unconditionally
returning the descriptor pointer as-is, regardless of whether the dummy
argument expects a descriptor. For bind(c) interfaces with assumed-size
dummies (e.g., cuFFT), the dummy expects a raw pointer, not a
descriptor. Passing the descriptor caused the C function to receive the
wrong address, leading to silent data corruption and invalid descriptor
crashes at deallocation.
The fix adds a check that the early return for ignore_tkr(c) only
applies when the dummy type is itself a descriptor type. When the dummy
expects a base address, the normal path is taken, which correctly
extracts the base address from the descriptor via fir.box_addr.
Fix https://github.com/llvm/llvm-project/issues/168561.
The issue was that the allocatable function result used as WHERE mask
was freed too early.
This happened because lowering was moving the result via as_expr but was
still inserting the deallocation directly on the storage instead of
passing that responsibility to the moved value (hlfir.expr). Later, the
pass optimizing WHERE is extending the expr lifetime usage passed the
deallocation of the storage because it (rightfully) assumes it can
extent lifetime of hlfir.expr as long as it moves the related destroy.
Fix lowering to pass the storage ownership to the expression after the
call instead.
This change moves the declare of result storage alloca before the call
so that alias analysis can revert to linking fir.declare to the fisrt
dominating dummy_scope instead of the dominating one.
This is only relevant when MLIR inlining is enabled and is the first
step to fix issues recent TBAA changes that placed target data in its
own tree exposed an issue with the result storage of a TARGET result.
After inlining, the usages of the result storage inside the callee and
after the call ended-up being placed in different nodes (target and non
target) of the same TBAA tree (for the dominating function).
The fact that both nodes are placed in the same tree stems from
https://github.com/llvm/llvm-project/pull/146006 that fixed another TBAA
issue related to MLIR inlining and function result where the function
result was placed into the wrong TBAA tree, which with nested inlining
could end-up being the tree of a callee where the result storage was a
dummy, causing the TBAA to wrongfully tell that any access to the result
storage inside the nested callee did not alias with any access after the
call.
By moving the declare before the call that will be inlined, this patch
will allow reverting #146006 and fixing both issues: the TBAA emit for
usages of the result storage after the call will always be placed in a
different TBAA tree than any usages of the result storage inside the
callee.
For descriptor arguments marked with `!dir$ ignore_tkr(c)` we want to
leave them unmodified: don't create another descriptor or change the
existing descriptor.
ISO Fortran now accepts a non-pointer actual argument to associate with
a dummy argument with the POINTER attribute if it is also INTENT(IN), so
long as the actual argument is a valid target for the pointer. But
passed-object dummy arguments still have a blanket prohibition against
being pointers in the ISO standard. Relax that constraint in the case of
INTENT(IN) so that passed objects can also benefit from the feature.
Fixes https://github.com/llvm/llvm-project/issues/172157.
This directive tells the compiler to ignore vector dependencies in the
following loop and it must be placed before a `do loop`.
Sometimes the compiler may not have sufficient information to decide
whether a particular loop is vectorizable due to potential dependencies
between iterations and the directive is here to tell to the compiler
that vectorization is safe with `parallelAccesses` metadata.
This directive is also equivalent to `#pragma clang loop assume(safety)`
in C++
Fixed the polymprphic check for copy-in/copy-out, added regression
tests.
Changed MayNeedCopy() to return std::optional<bool> and renamed it to
ActualArgNeedsCopy(). This function now returns true/false when it's
known that actual arguments needs copy in/out, or std::nullopt to
signify that it's now known, whether copy in/out is needed.
Fixes#159149
This patch adds the support of these two directives : `!dir$ inline` and
`!dir$ noinline`.
- `!dir$ noinline` tells to the compiler to not perform inlining on
specific function calls by adding the `noinline` metadata on the call.
- `!dir$ inline` tells to the compiler to attempt inlining on specific
function calls by adding the `inlinehint` metadata on the call.
- `!dir$ forceinline` tells to the compiler to always perfom inlining on
specific function calls by adding the `alwaysinline` metadata on the
call.
Currently, these directives can be placed before a `DO LOOP`, call
functions or assignments. Maybe other statements can be added in the
future if needed.
For the `inline` directive the correct name might be `forceinline` but
I'm not sure ?
When the stream is a scalar constant, it is lowered as i32. Stream needs
to be i64 to pass the verifier. Detect and update the stream reference
when it is i32.
The change adds a new HLFIR operation. A call to index intrinsic now
becomes lowered into the hlfir.index op and then naive lowering of the
op translates it back to appropriate runtime call. The change set is
aimed to be functionally equivalent to exiting index functionality, but
is much more efficient in a case of presence of the 'kind' intrinsic
parameter.
Also fixed couple of parameter lowering issues which were revealed while
working on the index-related functional parts.
Internal procedures expect reference parameters. However, when %VAL is
used, the argument should be passed by value. Forward the loaded address
and skip conversion to avoid a type conversion error.
Fixes#118239
Fixes https://github.com/llvm/llvm-project/issues/145151
Character elemental procedures require evaluating the result
specification expression outside of the elemental loops when the
function result length is not a constant. This is needed so that the
array result storage can be allocated before the evaluation if a storage
is needed.
Because the result specification expression may depend on inquires about
the dummy argument (but not usages of values thanks to F2023 C15121),
some representation of the dummy must be created. Since this is an
elemental call, this requires providing an element, and not the whole
array actual argument (we only care about the properties of such element
it does not matter which element is being used).
The previous code was creating the element with a type cast from the
base array, but this did not work with vector subscripted arrays where
the lowering representation is more complex. This caused a compiler
assert to fire.
Switch the code to only copy the properties that can be inquired from
the actual argument to the mock dummy (length parameters, dynamic type
and presence). A mock one address is used instead of addressing the
actual argument before the loop (one is used instead of NULL so that
presence inquiry will work as expected for OPTIONAL arguments).
New implementation of `MayNeedCopy()` is used to consolidate
copy-in/copy-out checks.
`IsAssumedShape()` and `IsAssumedRank()` were simplified and are both
now in `Fortran::semantics` workspace.
`preparePresentUserCallActualArgument()` in lowering was modified to use
`MayNeedCopyInOut()`
Fixes https://github.com/llvm/llvm-project/issues/138471
ArrayRef(std::nullopt_t) has been deprecated. This patch replaces
std::nullopt with {}.
A subsequence patch will address those places where we need to replace
std::nullopt with mlir::TypeRange{} or mlir::ValueRange{}.
Some MPI libraries use character dummies + ignore(TKR) to allow passing
any kind of buffer.
This was meant to already be handled by #108168
However, when the library interface also had an argument requiring an
explicit interface, `builder.convertWithSemantics` was not allowed to properly deal
with the actual/dummy type mismatch and generated bad IR causing errors like:
`'fir.convert' op invalid type conversion'!fir.ref' / '!fir.boxchar\<1\>'`.
This restriction was artificial, lowering should just handle any cases
allowed by semantics. Just remove it.
If we generate hlfir.end_associate at the end of the statement,
we get easier optimizable HLFIR, because there are no compiler
generated operations with side-effects in between the call
and the consumers. This allows more hlfir.eval_in_mem to reuse
the LHS instead of allocating temporary buffer.
I do not think the same can be done for hlfir.copy_out always, e.g.:
```
subroutine test2(x)
interface
function array_func2(x,y)
real:: x(*), array_func2(10), y
end function array_func2
end interface
real :: x(:)
x = array_func2(x, 1.0)
end subroutine test2
```
If we postpone the copy-out until after the assignment, then
the result may be wrong.
[RFC on
discourse](https://discourse.llvm.org/t/rfc-volatile-representation-in-flang/85404/1)
Flang currently lacks support for volatile variables. For some cases,
the compiler produces TODO error messages and others are ignored. Some
of our tests are like the example from _C.4 Clause 8 notes: The VOLATILE
attribute (8.5.20)_ and require volatile variables.
Prior commits:
```
c9ec1bc753b0 [flang] Handle volatility in lowering and codegen (#135311)
e42f8609858f [flang][nfc] Support volatility in Fir ops (#134858)
b2711e1526f9 [flang][nfc] Support volatile on ref, box, and class types (#134386)
```
Update `cuf.kernel_launch` to take the stream as a reference. Update the
conversion to insert the `cuf.stream_cast` op so the stream can be set
as dependency.
* Enable lowering and conversion patterns to pass volatility information
from higher level operations to lower level ones.
* Enable codegen to pass volatility to LLVM dialect ops by setting an
attribute on loads, stores, and memory intrinsics.
* Add utilities for passing along the volatility from an input type to
an output type.
To introduce volatile types into the IR, entities with the volatile
attribute will be given a volatile type in the bridge; this is not
enabled in this patch. User code should not result in IR with volatile
types yet, so this patch contains no tests with Fortran source, only IR
that already contains volatile types.
Part 3 of #132486.
IsSymplyContiguous was visiting expressions and returning false on
expressions like `x(::2) + y`, which triggered an assert in lowering
when preparing arguments for copy-in/out.
Update it to return false for everything that is not a variable, except
when provided a flag to treat PARAMETER bases as variables. This flags
is required for internal usages in lowering where lowering needs to now
if the read-only memory is being addressed contiguously or not.
Update call lowering to always copy parameter array section into
contiguous writable memory when passing them. The rational here is that
copy-out generated in nested calls using the dummy arguments will cause
a segfault.
This patch shares core interface methods dealing with argument and
result attributes from CallableOpInterface with the CallOpInterface and
makes them mandatory to gives more consistent guarantees about concrete
operations using these interfaces.
This allows adding argument attributes on call like operations, which is
sometimes required to get proper ABI, like with llvm.call (and llvm.invoke).
The patch adds optional `arg_attrs` and `res_attrs` attributes to operations using
these interfaces that did not have that already.
They can then re-use the common "rich function signature"
printing/parsing helpers if they want (for the LLVM dialect, this is
done in the next patch).
Part of RFC: https://discourse.llvm.org/t/mlir-rfc-adding-argument-and-result-attributes-to-llvm-call/84107
This patch encapsulate array function call lowering into
hlfir.eval_in_mem and allows directly evaluating the call into the LHS
when possible.
The conditions are: LHS is contiguous, not accessed inside the function,
it is not a whole allocatable, and the function results needs not to be
finalized. All these conditions are tested in the previous hlfir.eval_in_mem
optimization (#118069) that is leveraging the extension of getModRef to
handle function calls(#117164).
This yields a 25% speed-up on polyhedron channel2 benchmark (from 1min
to 45s measured on an X86-64 Zen 2).
This alternative loop nest generation is used to generate an OpenMP loop nest instead of fir loops to facilitate parallelizing statements in an OpenMP `workshare` construct.
Currently, it is not possible to distinguish between BIND(C) from
non-BIND(C) type bound procedure call at the FIR level.
This will be a problem when dealing with derived type BIND(C) function
where the ABI differ between BIND(C)/non-BIND(C) but the FIR signature
looks like the same at the FIR level.
Fix this by adding the Fortran procedure attributes to fir.distpatch,
and propagating it until the related fir.call is generated in
fir.dispatch codegen.
Follow-up from a previous patch that turned bind_c into an enum for
procedure attribute.
This patch carries the elemental and pure Fortran attribute into FIR so
that the optimizer can leverage that info in the future (I think debug
info may also need to know these aspects since DWARF has DW_AT_elemental
and DW_AT_pure nodes).
SIMPLE from F2023 will be translated once it is handled in the
front-end.
NON_RECURSIVE is only meaningful on func.func since we are not
guaranteed to know that aspect on the caller side (it is not part of
Fortran characteristics). There is a DW_AT_recursive DWARF node. I will
do it while dealing with func.func attributes.
stack save/restore emitted for character elemental function result
allocation inside hlfir.elemental in lowering created memory bugs
because result memory is actually still used after the stack restore
when lowering the elemental into a loop where the result element is
copied into the array result storage.
Instead of adding special handling for stack save/restore in lowering,
just avoid emitting those since the stack reclaim pass is able to emit
them in the generated loop. Not having those stack save/restore will
also help optimizations that want to elide the temporary allocation for
the element result when that is possible.
The test code with ignore_tkr(tk) on character dummy passed by
fir.boxchar<> was crashing the compiler in [an
assert](2afe678f0a/flang/lib/Optimizer/Dialect/FIRType.cpp (L632))
in `changeElementType`.
It makes little sense to call changeElementType on a fir.boxchar since
this type is lossy (the shape is not part of it). Just skip it in the
code dealing with ignore(tk) when hitting this case
The new LLVM stack save/restore intrinsic operations are more convenient
than function calls because they do not add function declarations to the
module and therefore do not block the parallelisation of passes.
Furthermore they could be much more easily marked with memory effects
than function calls if that ever proved useful.
This builds on top of #107879.
Resolves#108016
First patch to fix a BIND(C) ABI issue
(https://github.com/llvm/llvm-project/issues/102113). I need to keep
track of BIND(C) in more locations (fir.dispatch and func.func
operations), and I need to fix a few passes that are dropping the
attribute on the floor. Since I expect more procedure attributes that
cannot be reflected in mlir::FunctionType will be needed for ABI,
optimizations, or debug info, this NFC patch adds a new enum attribute
to keep track of procedure attributes in the IR.
This patch is not updating lowering to lower more attributes, this will
be done in a separate patch to keep the test changes low here.
Adding the attribute on fir.dispatch and func.func will also be done in
separate patches.
When passing a polymorphic actual array argument to an non polymorphic
explicit or assumed shape argument, copy-in/copy-out may be required and
should be made according to the dummy dynamic type.
The code that was creating the descriptor to drive this copy-in/out was
not handling properly the case where the dummy and actual rank do not
match (possible according to sequence association rules), it tried to
make the copy-in/out according to the dummy argument shape (which we may
not even know if the dummy is assumed-size). Fix this by using the
actual shape when creating this new descriptor with the dummy argument
dynamic type.
Until genSecond, all intrinsic `genXXX` returning scalar intrinsic
(except NULL) were returning them as value.
The code calling genIntrinsicCall is using that assumption when
generation the asExprOp because hflir.expr<> of scalar are badly
supported in tools (I should likely just forbid them all together), the
type is meant for "non trivial" values: arrays, character, and derived
type. For instance, the added tests crashed with error: `'arith.subf' op
operand #0 must be floating-point-like, but got '!hlfir.expr<f32>'`
Load the result in genSecond and add an assert after genIntrinsicCall to
better enforce this.
Currently, all procedures from intrinsic modules that are not BIND(C)
are expected to be intercepted by the compiler in lowering and to have a
handler in IntrinsicCall.cpp.
As more "intrinsic" modules are being added (OpenMP, OpenACC, CUF, ...),
this requirement is preventing seamless implementation of intrinsic
modules in Fortran. Procedures from intrinsic modules are different from
generic intrinsics defined in section 16 of the standard. They are
declared in Fortran file seating in the intrinsic module directory and
inside the compiler they look like regular user call except for the
INTRINSIC attribute set on their module. So an easy implementation is
just to have the implementation done in Fortran and linked to the
runtime without any need for the compiler to necessarily understand and
handle these calls in special ways.
This patch splits the lookup and generation part of IntrinsicCall.cpp so
that it can be allowed to only intercept calls to procedure from
intrinsic module if they have a handler. Otherwise, the assumption is
that they should be implemented in Fortran.
Add explicit TODOs handler for the IEEE procedure that are known to not
yet been implemented and won't be implemented via Fortran code so that
this patch is an NFC for what is currently supported.
This patch also prevents doing two lookups in the intrinsic table (There
was one to get argument lowering rules, and another one to generate the
code).
Just remove the TODO and add a test.
There is nothing special to do to deal with assumed-rank copy-in/out
after the previous copy-in/out API change in
https://github.com/llvm/llvm-project/pull/95822.
The runtime API for copy-in copy-out currently only has an entry only
for the copy-out. This entry has a "skipInit" boolean that is never set
to false by lowering and it does not deal with the deallocation of the
temporary.
The generated code was a mix of inline code and runtime calls This is not a big deal,
but this is unneeded compiler and generated code complexity.
With assumed-rank, it is also more cumbersome to establish a
temporary descriptor.
Instead, this patch:
- Adds a CopyInAssignment API that deals with establishing the temporary
descriptor and does the copy.
- Removes unused arg to CopyOutAssign, and pushes
destruction/deallocation responsibility inside it.
Note that this runtime API are still not responsible for deciding the
need of copying-in and out. This is kept as a separate runtime call to
IsContiguous, which is easier to inline/replace by inline code with the
hope of removing the copy-in/out calls after user function inlining.
@vzakhari has already shown that always inlining all the copy part
increase Fortran compilation time due to loop optimization attempts for
loops that are known to have little optimization profitability (the
variable being copied from and to is not contiguous).
Deal with the cases where lower bounds, or attribute, or dynamic type
must be updated when passing an assumed-rank actual argument to an
assumed-rank dummy argument.
copy-in/copy-out and passing target assumed-rank to intent(in) pointers
will be handled in separate patch.
Prepare the argument and map them to their corresponding dummy symbol in
order to lower the specification expression of the function result.
Extract the preparation of arguments according to the interface to its
own function to be reused.
It seems there is no need to conditionally compute the length on the
input since all the information comes from the CharBoxValue or the
descriptor for cases where the number of element could be 0.
The BIND(C) attribute attached to a function can be lost when we do
indirect call. This information might be useful for codegen that have
specific ABI. This patch carry over the BIND(C) information to the
fir.call operation.
Handle lowering of non optional inquired argument in custom lowering.
Also fix an issue in the lowering of associated optional argument where
a box was emboxed again which led to weird result.
