User Details
- User Since
- Mar 16 2020, 6:35 AM (43 w, 5 d)
Fri, Jan 8
Looks good to me, thanks Pete.
Thu, Jan 7
@schweitz, as I understand things, the current lowering code gets the value of alternate return labels from the parse tree. Do you plan to change that once their available in the values of the ActualArgument?
That would make sens IMHO.
Otherwise, the change you have should work OK with lowering, except for the ProcedureRef hasAlternateReturns that is now wrong and would make lowering fail (see inlined comment).
Dec 16 2020
Looks good to me
Dec 8 2020
Nov 16 2020
Looks good to me.
Nov 12 2020
Thanks, looks good to me.
Nov 10 2020
Looks good to me
Nov 2 2020
Looks good to me
Looks good me
Oct 26 2020
Thanks for the reviews. Fix typos in documentation an error message.
Oct 23 2020
Oct 20 2020
Oct 19 2020
Only allow INTENT(OUT) allocatable coarrays in intrinsics where they are
explicitly allowed intstead of all intrinsics.
Oct 16 2020
Change DummyArgument::HasIntent to DummyArgument::GetIntent.
Oct 15 2020
Oct 14 2020
LGTM
The patch compiles successfully with msvc (with a patch to trunk that I still need to upload a patch for).
Thanks for testing this @Meinersbur !
I can build and run tests as expected with and without LLVM_LINK_LLVM_DYLIB now. Thanks for making flang work with this build option !
Oct 13 2020
@Meinersbur, I have added you as a reviewer since this patch is touching a lot of template/constexpr features so it is likely it will hit MSVC errors. If you have the opportunity to test it ahead that would be great, otherwise beware that it may lead to MSVC regressions.
Thanks for the reviews, I have updated the patch where I could.
Remove static_assert from StaticMultimapView
- Simplify StaticMultimapView (use std::equal_range and removed unused member functions)
- Add REAL(2) and REAL(3) folding test
- Enable X87 float folding with IEEE 128 bits float runtime if available on the host.
- Use RealKindForPrecision instead of TypeOf
- Add a compile time verfication that StaticMultimapView is sorted
- Capitalize some functions. std::nullopt instead of {}.
- Use raw subnormal in flushing test instead of decimal input.
Oct 12 2020
Thanks for this update, I just finished testing it. There is one issue with a really really slow test being run in regression (see inline comment). Apart from that, this patch built in my environment with/without LLVM_LINK_LLVM_DYLIB in/out of LLVM tree.
LGTM (genObjectListWithModifier is redundant with the one in https://reviews.llvm.org/D88918, I suppose you will handle this conflict while merging ? ).
LGTM
LGTM
Oct 7 2020
Looks good to me
Looks good to me.
Oct 6 2020
Looks good to me.
Sep 23 2020
Thanks for fixing this. I tested in my build environment. I could reproduce the failure without this fix at build time with LLVM_LINK_LLVM_DYLIB when Generating ../../include/flang/__fortran_builtins.mod. With this fix, I can still build OK without LLVM_LINK_LLVM_DYLIB. With LLVM_LINK_LLVM_DYLIB, flang now builds, but I see 14 flang regression tests are failing with error CommandLine Error: Option 'disable-symbolication' registered more than once!. I think it may be a test linking issue that is remaining, but at least LLVM_LINK_LLVM_DYLIB now works to build flang.
LGTM regarding my comment for the unit test.
Sep 22 2020
Jul 31 2020
Jul 30 2020
Looks good to me, thanks for implementing this.
Jul 29 2020
Jul 24 2020
In this very spot, the goal of the template is to digest the runtime headers in order to lower their function signature to an mlir::FuncOp declaration that is compatible with the runtime and will be call in user programs.
So if we use std::complex here and introduce a wrapper indirection in C around the runtime, then the cost of this indirection will be paid by the user program. I am not comfortable with having a Fortran programmer pay the price of a C/C++ compatibility issues.
Jul 23 2020
Sorry I was harvesting potatoes far from any computers when all these discussions happened. First of all, thanks for detecting the warning and proposing a fix for it. I will try to explain our issue and why we cannot use safely std::complex here so we can find a fix for this.
Jul 3 2020
LGTM, thanks for doing this.
LGTM
Jul 2 2020
Removed old FIXME that was actually implemented in fold-real.cc
Jun 25 2020
Jun 23 2020
Jun 22 2020
LGTM, thanks
Why not updating semantics::IsSaved to test IsFunctionResult instead like it is doing with IsDummy ?
Lowering relies on IsSaved to indicate whether something is implicitly or explicitly saved, so with this patch it looks like we would still create a global for f2b even if we should not.
Jun 19 2020
Jun 17 2020
Add comments in AMAX0/MIN1... rewrite algorithm.
Jun 16 2020
LGTM, unit test can always be added later when the framework is ready.
Jun 4 2020
Jun 3 2020
May 6 2020
Thanks for adding these examples
Apr 29 2020
Apr 27 2020
Would it make sens to also upstream https://github.com/flang-compiler/f18-llvm-project/blob/fir-dev/flang/documentation/BijectiveInternalNameUniquing.md in this patch. It explains the rational of this mangling.
Otherwise looks good to me.
Apr 16 2020
Looks like originalFenv_ is still used for restoring the original state. Shall I change the type of originalFenv_ to be unsigned int on x86_64? (Or maybe a union?)
From what I understand originalFenv_ is used for changing and restoring the __mxcsr value (or its equivalents on aarch64).
In host.cpp, originalFenv_ is more than just the __mxcsr, it also backs-up the whole fp environment (original rounding mode, exceptions that were raised,...) before calling the run-time to fold. Interrupts are also disabled between the related feholdexcept / fesetenv (we do not want the compiler to crash if the Fortran code being folded is wrong and triggers a division by zero or such), so it should be kept.
currentFenv_ however was only about manipulating the __mxcsr (or __fpcr /...), so you can remove it where you do not need it anymore.