[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic closed this revision.
jpenix-quic added a comment.

0ec3ac9b7fbd 


Gah, I goofed and forgot to add the "Differential Revision" line--this has been 
committed at 0ec3ac9b7fbd15698af7289e1214e8ff3d82ec14 
.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.

Ping @awarzynski and @klausler--could you please let me know if the driver and 
runtime portions look good to you, respectively? (Apologies in advance if this 
isn't necessary, but as you both gave me significant feedback I wanted to make 
sure you are ok with the patch as it stands.) Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic marked an inline comment as done.
jpenix-quic added a comment.

In D130513#3827108 , @jeanPerier 
wrote:

> The lowering part looks good to me (I only have a minor comment inlined about 
> a header used in lowering).

Thank you @jeanPerier for looking over the lowering portion! Regarding moving 
the header (I'm replying to the comment here since the inline one now opens in 
a separate revision/window as the file is gone), I moved it to 
Lower/EnvironmentDefault.h as lowering is where I use it (as you mentioned). 
Please let me know if this needs further changes!

Just to confirm/as a final check, @awarzynski does the driver portion look good 
to you? Similarly, @klausler does the runtime portion look good to you? I 
believe I addressed the comments you both left previously, but I want to make 
sure that I addressed things appropriately. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 464849.
jpenix-quic added a comment.

Add /*overwrite=*/ comment I missed previously, move 
Runtime/environment-defaults.h to Lower/EnvironmentDefault.h


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Lower/EnvironmentDefault.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -224,7 +224,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, semanticsContext, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+
+! CHECK: fir.global @_QQEnvironmentDefaults constant : !fir.ref, !fir

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.

> The build still fails.

I think it might be an infrastructure issue or something like that--I've tried 
restarting it a few times and each time it just ends with "HTTP 28" as the only 
message I see. Another review that was created a bit ago 
(https://reviews.llvm.org/D134329) seems to have the same issue, so I'll try 
restarting it again in a while.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic marked 5 inline comments as done.
jpenix-quic added inline comments.



Comment at: flang/test/Driver/convert.f90:1
+! Ensure argument -fconvert= works as expected.
+

awarzynski wrote:
> rovka wrote:
> > Nit: Shouldn't you also check that valid values (e.g. unknown, swap etc) 
> > are accepted? 
> Could you be more specific? IIUC, this is more about making sure that the 
> option parser works correctly and reports invalid usage of `-fconvert` as an 
> error, right?
> Nit: Shouldn't you also check that valid values (e.g. unknown, swap etc) are 
> accepted? 

I might be doing something silly/missing something obvious, but I wasn't sure 
how to get FileCheck to basically just check that the command didn't error 
without adding checks, so I added trivial checks (VALID/VALID_FC1) for each of 
unknown/native/etc. Is there a better way of doing this? I also expanded 
flang/test/Lower/convert.f90 to check lowering for each of the valid options, 
so I didn't want to do full checks of the MLIR here. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 461759.
jpenix-quic marked an inline comment as done.
jpenix-quic added a comment.

Move comment, see if build will cooperate.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -222,7 +222,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, semanticsContext, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+
+! CHECK: fir.global @_QQEnvironmentDefaults constant : !fir.ref, !fir.ref> {
+! CHECK: 

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 461753.
jpenix-quic added a comment.

Address comments from @awarzynski, @rovka, and @peixin. Also fixed the file 
header comments for EnvironmentDefaults.h, environment-defaults.h, and 
environment-default-list.h to match others in their respective folders.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -222,7 +222,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, semanticsContext, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environ

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added inline comments.



Comment at: flang/test/Driver/emit-mlir.f90:16
 ! CHECK-NEXT: }
+! CHECK-NEXT: fir.global @_QQEnvironmentDefaults constant : 
!fir.ref, 
!fir.ref> {
+! CHECK-NEXT:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref, !fir.ref>

peixin wrote:
> jpenix-quic wrote:
> > peixin wrote:
> > > jpenix-quic wrote:
> > > > peixin wrote:
> > > > > Is it possible not to generated this global variable if `fconvert=` 
> > > > > is not specified?
> > > > I'm not entirely sure--the issue I was running into was how to handle 
> > > > this in Fortran_main.c in a way which worked for all of 
> > > > GCC/Clang/Visual Studio (and maybe others?). I was originally thinking 
> > > > of doing this by using a weak definition of _QQEnvironmentDefaults set 
> > > > to nullptr so fconvert, etc. could override this definition without 
> > > > explicitly generating the fallback case. For GCC/clang, I think I could 
> > > > use __attribute__((weak)), but I wasn't sure how to handle this if 
> > > > someone tried to build with Visual Studio (or maybe another toolchain). 
> > > > I saw a few workarounds (ex: 
> > > > https://devblogs.microsoft.com/oldnewthing/20200731-00/?p=104024) but I 
> > > > shied away from this since it seems to be an undocumented feature (and 
> > > > presumably only helps with Visual Studio). 
> > > > 
> > > > Do you know of a better or more general way I could do this? (Or, is 
> > > > there non-weak symbol approach that might be better that I'm missing?)
> > > How about generate one runtime function with the argument of 
> > > `EnvironmentDefaultList`? This will avoid this and using one extern 
> > > variable?
> > > 
> > > If users use one variable with bind C name `_QQEnvironmentDefaults` in 
> > > fortran or one variable with name `_QQEnvironmentDefaults` in C, it is 
> > > risky. Would using the runtime function and static variable with the type 
> > > `EnvironmentDefaultList` in runtime be safer?
> > Agreed that there are potential risks with the current approach (although, 
> > are the `_Q*` names considered reserved?). Unfortunately, I think 
> > generating a call to set the environment defaults requires somewhat 
> > significant changes to the runtime. The runtime reads environment variables 
> > during initialization in `ExecutionEnvironment::Configure` which is 
> > ultimately called from the "hardcoded" `Fortran_main.c` and I need to set 
> > the defaults before this happens. So, I believe I'd either have to move the 
> > initialization to `_QQmain`  or make it so that `main` isn't hardcoded so 
> > that I could insert the appropriate runtime function.
> > 
> > @klausler I think I asked you about this when I was first trying to figure 
> > out how to implement the environment defaults and you suggested I try the 
> > extern approach--please let me know if you have thoughts/suggestions around 
> > this!
> This is what @klausler suggested:
> ```
> Instead of adding new custom APIs that let command-line options control 
> behavior in a way that is redundant with the runtime environment, I suggest 
> that you try a more general runtime library API by which the main program can 
> specify a default environment variable setting, or a set of them. Then turn 
> the command-line options into the equivalent environment settings and pass 
> them as default settings that could be overridden by the actual environment.
> ```
> If I understand correctly, what I am suggesting match his comments. The "main 
> program" he means should be fortran main program, not the 
> `RTNAME(ProgramStart`. In your initial patch, you add the runtime specified 
> for "convert option". I think @klausler suggest you making the runtime 
> argument more general used for a set of runtime environment variable 
> settings, not restricted to "convert option". And that is what you already 
> added -- `EnvironmentDefaultList`. So, combining this patch and your initial 
> patch will be the solution. Hope I understand it correctly.
The issue I hit with the suggested approach is that in order to use the 
pre-existing runtime environment variable handling to set the internal state I 
need to set the environment variable defaults before the environment variables 
are read by the runtime.

I might be misunderstanding/missing something, but given that the environment 
variables are read as part of `RTNAME(ProgramStart)` in `main` and the earliest 
I can place the call if I am generating it is `_QQmain`, I think that leaves 
three options: 1. don't hardcode `main` so that I can place the call early 
enough 2. delay or rerun the code [[ 
https://github.com/llvm/llvm-project/blob/c619d4f840dcba54751ff8c5aaafce0f173a4ad5/flang/runtime/environment.cpp#L50-L90
 | here ]] that is responsible for initializing the runtime state so that it is 
called as part of `_QQmain` so I can insert my runtime function before it or 3. 
hardcode something like the `_QQEnvironmentDefaults` into Fortran_main.c so 

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic marked 2 inline comments as done.
jpenix-quic added inline comments.



Comment at: flang/runtime/environment.cpp:77
+#else
+  envp = environ;
+#endif

peixin wrote:
> What is `environ` used for? Should we try to keep as less extern variable as 
> possible in runtime for security.
I think `setenv` is only required to make sure that the `environ` pointer 
points to the correct copy of the environment variables. So, as long as we are 
storing a copy of the environment pointer in `ExecutionEnvironment` and 
potentially making changes to the environment via `setenv`, I think we need to 
base it off the `environ` pointer after `setenv` has been called rather than 
the `envp` from `main`.

That said, I don't think the `envp` variable we are storing is being used for 
anything at the moment (reading environment variables was changed to use 
`getenv` rather than `envp` in the commit [[ 
https://github.com/llvm/llvm-project/commit/824bf908194c9267f1f09065ba8e41d7969006ab
 | here]]). If removing the usage of `environ` is preferable, we could maybe 
remove the usage of `envp` altogether (in a separate patch)--does this sound 
like a good idea or would it be better to just leave in the `environ` usage for 
now?



Comment at: flang/test/Driver/emit-mlir.f90:16
 ! CHECK-NEXT: }
+! CHECK-NEXT: fir.global @_QQEnvironmentDefaults constant : 
!fir.ref, 
!fir.ref> {
+! CHECK-NEXT:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref, !fir.ref>

peixin wrote:
> jpenix-quic wrote:
> > peixin wrote:
> > > Is it possible not to generated this global variable if `fconvert=` is 
> > > not specified?
> > I'm not entirely sure--the issue I was running into was how to handle this 
> > in Fortran_main.c in a way which worked for all of GCC/Clang/Visual Studio 
> > (and maybe others?). I was originally thinking of doing this by using a 
> > weak definition of _QQEnvironmentDefaults set to nullptr so fconvert, etc. 
> > could override this definition without explicitly generating the fallback 
> > case. For GCC/clang, I think I could use __attribute__((weak)), but I 
> > wasn't sure how to handle this if someone tried to build with Visual Studio 
> > (or maybe another toolchain). I saw a few workarounds (ex: 
> > https://devblogs.microsoft.com/oldnewthing/20200731-00/?p=104024) but I 
> > shied away from this since it seems to be an undocumented feature (and 
> > presumably only helps with Visual Studio). 
> > 
> > Do you know of a better or more general way I could do this? (Or, is there 
> > non-weak symbol approach that might be better that I'm missing?)
> How about generate one runtime function with the argument of 
> `EnvironmentDefaultList`? This will avoid this and using one extern variable?
> 
> If users use one variable with bind C name `_QQEnvironmentDefaults` in 
> fortran or one variable with name `_QQEnvironmentDefaults` in C, it is risky. 
> Would using the runtime function and static variable with the type 
> `EnvironmentDefaultList` in runtime be safer?
Agreed that there are potential risks with the current approach (although, are 
the `_Q*` names considered reserved?). Unfortunately, I think generating a call 
to set the environment defaults requires somewhat significant changes to the 
runtime. The runtime reads environment variables during initialization in 
`ExecutionEnvironment::Configure` which is ultimately called from the 
"hardcoded" `Fortran_main.c` and I need to set the defaults before this 
happens. So, I believe I'd either have to move the initialization to `_QQmain`  
or make it so that `main` isn't hardcoded so that I could insert the 
appropriate runtime function.

@klausler I think I asked you about this when I was first trying to figure out 
how to implement the environment defaults and you suggested I try the extern 
approach--please let me know if you have thoughts/suggestions around this!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 460819.
jpenix-quic added a comment.

Remove unneeded braces and unnecessary changes to NameUniquer.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -222,7 +222,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, semanticsContext, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,11 @@
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+
+! CHECK: fir.global @_QQEnvironmentDefaults constant : !fir.ref, !fir.ref> {
+! CHECK:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref, !fir.ref>
+! CHECK: fir.has_value

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added inline comments.



Comment at: flang/test/Driver/emit-mlir.f90:16
 ! CHECK-NEXT: }
+! CHECK-NEXT: fir.global @_QQEnvironmentDefaults constant : 
!fir.ref, 
!fir.ref> {
+! CHECK-NEXT:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref, !fir.ref>

peixin wrote:
> Is it possible not to generated this global variable if `fconvert=` is not 
> specified?
I'm not entirely sure--the issue I was running into was how to handle this in 
Fortran_main.c in a way which worked for all of GCC/Clang/Visual Studio (and 
maybe others?). I was originally thinking of doing this by using a weak 
definition of _QQEnvironmentDefaults set to nullptr so fconvert, etc. could 
override this definition without explicitly generating the fallback case. For 
GCC/clang, I think I could use __attribute__((weak)), but I wasn't sure how to 
handle this if someone tried to build with Visual Studio (or maybe another 
toolchain). I saw a few workarounds (ex: 
https://devblogs.microsoft.com/oldnewthing/20200731-00/?p=104024) but I shied 
away from this since it seems to be an undocumented feature (and presumably 
only helps with Visual Studio). 

Do you know of a better or more general way I could do this? (Or, is there 
non-weak symbol approach that might be better that I'm missing?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added inline comments.



Comment at: flang/runtime/environment-default-list.h:1
+//===-- Runtime/environment-default-list.h 
===//
+//

klausler wrote:
> If you want this header to be maximally portable C and C++, please observe 
> the usage of other such headers, and use old-school C /*comments*/.
> 
> You could probably just use an 'int' for the item count and avoid some 
> difficulty below, unless you expect a program to use billions of default 
> environment settings.
Done re: using an int!

Just to double check regarding C/C++ portability and looking at other headers, 
one that I was looking at was Decimal/decimal.h and the structs, etc. in that 
file are conditionally added to namespaces depending on whether it is C or C++. 
I was waffling on whether I should be doing that here though (I am not 
currently/was not previously) as keeping the type out of the namespace allows 
me to keep a consistent type/directly pass the pointer from Fortran_main.c to 
main.cpp/enviornment.cpp. But, as a result I'm also polluting the default 
namespace with EnvironmentDefaultList/Item for C++ code. Is how I have it 
currently ok, or would it be better to move the structs into namespaces for C++ 
and just cast to the correct type along the way? (Or, is there another option I 
am missing?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 455975.
jpenix-quic added a comment.

Fixed up/simplified environment-default-list.h for C/C++ compatibility. Also 
cleaned up a declaration and a few autos in the lowering component. Rebased.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Optimizer/Support/InternalNames.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/lib/Optimizer/Support/InternalNames.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -222,7 +222,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, semanticsContext, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,11 @@
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults 

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.

@klausler Could you please take a look at this again and let me know if it is 
more in line with your suggestion above?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 451225.
jpenix-quic added a comment.

Rebase to address conflicts, use string substitution blocks in my tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Lower/Runtime.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Optimizer/Support/InternalNames.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/lib/Optimizer/Support/InternalNames.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -222,7 +222,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap, loweringOptions);
+  kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,11 @@
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+
+! CHECK: fir.global @_QQEnvironment

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added inline comments.



Comment at: flang/lib/Lower/Bridge.cpp:203
 //them.
 for (Fortran::lower::pft::Program::Units &u : pft.getUnits()) {
   std::visit(Fortran::common::visitors{

peixin wrote:
> Doing this can avoid add one variable to the bridge.
Done! (Although, I added the `isMainProgram()` check/update to the lambda below 
as it is a member function of `FunctionLikeUnit` specifically--please let me 
know if this looks ok!)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 450895.
jpenix-quic edited the summary of this revision.
jpenix-quic added a comment.

Change hasMainProgram to be a local variable and update summary to reflect the 
new approach.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Lower/Runtime.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Optimizer/Support/InternalNames.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/lib/Optimizer/Support/InternalNames.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -220,7 +220,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap);
+  kindMap, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,12 @@
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 449894.
jpenix-quic added a comment.

Hopefully fix Window's build errors by using the proper Windows-specific 
environment utilities (_putenv_s vs setenv).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Lower/Runtime.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Optimizer/Support/InternalNames.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/lib/Optimizer/Support/InternalNames.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(
+  0, nullptr, nullptr, nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -220,7 +220,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap);
+  kindMap, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(
+int, const char *[], const char *[], const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStart)(0, 0, 0);
+  RTNAME(ProgramStart)(0, 0, 0, 0);
   int32_t c = RTNAME(ArgumentCount)();
   int32_t v = RTNAME(GetCommandArgument)(0, 0, 0, 0);
   return x + c + v;
Index: flang/test/Lower/environment-defaults.f90
===
--- /dev/null
+++ flang/test/Lower/environment-defaults.f90
@@ -0,0 +1,12 @@
+! RUN: bbc -emit-fir -o - %s | FileCheck %s
+
+program test
+  continue
+end
+
+! Test that a null pointer is generated for environment defaults if nothing is specified
+
+! CHECK: fir.global @_Q

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.



> BTW, can you continue working on the lowering of the convert argument of open 
> statement?

I will take a look at this!




Comment at: flang/lib/Lower/Bridge.cpp:2757
 
+if (funit.isMainProgram()) {
+  auto conversion = bridge.getConversion();

peixin wrote:
> I think this should be moved into `void 
> lowerFunc(Fortran::lower::pft::FunctionLikeUnit &funit)`.
I ended moving this again while changing approaches--the equivalent line is 
above at lines 243-252. Does the new location seem reasonable? I had to add the 
bool to indirectly track if a main program was specified, but the new location 
seemed the most fitting to me. 



Comment at: flang/runtime/main.cpp:55
+
+  if (auto convert{Fortran::runtime::GetConvertFromInt(i)}) {
+Fortran::runtime::executionEnvironment.conversion = *convert;

peixin wrote:
> Nit: Is it better to to check the range of i before getting it from 
> `GetConvertFromInt`?
> ```
> if (i < static_cast(Convert::Unknown) || i > 
> static_cast(Convert::Swap)) {
>   crash
> } else {
>   Fortran::runtime::executionEnvironment.conversion = static_cast(i);
> }
> ```
I ended up not needing `GetConvertFromInt` when reworking the approach, so this 
has been removed!



Comment at: flang/runtime/main.cpp:58
+  } else {
+Fortran::runtime::Terminator{__FILE__, __LINE__}.Crash(
+"Invalid convert option passed to ConvertOption");

peixin wrote:
> Should `__FILE__, __LINE__` be passed as argument in lowering to point to the 
> file name and line of the source file? Or is this (__FILE__, __LINE__) 
> pointing the the file name and line of the source file?
I removed this instance of `__FILE__, __LINE__`, but I added another similar 
one in `environment.cpp` line 33! 

The error points to `environment.cpp` line 33 in the new revision. I'm not sure 
if this is the best way of handling it, but given that there isn't a Fortran 
source line associated with either the environment list or the old runtime call 
(and as I think both are closer to implementation details than user-facing 
features), pointing to the runtime file seemed like the least confusing option 
for the error location.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 449875.
jpenix-quic added a comment.
Herald added subscribers: bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, 
teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester, arpith-jacob, 
nicolasvasilache, antiagainst, shauheen, rriddle.

Attempted to address comments suggesting a different approach for the 
implementation. Rather than add a call to a new set_convert() runtime function, 
create a list of environment variable defaults that is passed into and set by 
the runtime via a "known" extern data structure. Also rebased.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/examples/external-hello.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Lower/Runtime.h
  flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
  flang/include/flang/Optimizer/Support/InternalNames.h
  flang/include/flang/Runtime/environment-defaults.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
  flang/lib/Optimizer/Support/InternalNames.cpp
  flang/runtime/FortranMain/Fortran_main.c
  flang/runtime/environment-default-list.h
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/emit-mlir.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Lower/convert.f90
  flang/test/Lower/environment-defaults.f90
  flang/test/Runtime/no-cpp-dep.c
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/CommandTest.cpp
  flang/unittests/Runtime/Stop.cpp

Index: flang/unittests/Runtime/Stop.cpp
===
--- flang/unittests/Runtime/Stop.cpp
+++ flang/unittests/Runtime/Stop.cpp
@@ -26,7 +26,8 @@
 
 TEST(TestProgramEnd, StopTestNoStopMessage) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr,
+   nullptr);
   EXPECT_EXIT(
   RTNAME(StopStatement)(), testing::ExitedWithCode(EXIT_SUCCESS), "");
 }
@@ -52,7 +53,8 @@
 
 TEST(TestProgramEnd, NoStopMessageTest) {
   putenv(const_cast("NO_STOP_MESSAGE=1"));
-  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr);
+  Fortran::runtime::executionEnvironment.Configure(0, nullptr, nullptr,
+   nullptr);
   static const char *message{"bye bye"};
   EXPECT_EXIT(RTNAME(StopStatementText)(message, std::strlen(message),
   /*isErrorStop=*/false, /*quiet=*/false),
Index: flang/unittests/Runtime/CommandTest.cpp
===
--- flang/unittests/Runtime/CommandTest.cpp
+++ flang/unittests/Runtime/CommandTest.cpp
@@ -49,7 +49,7 @@
 class CommandFixture : public ::testing::Test {
 protected:
   CommandFixture(int argc, const char *argv[]) {
-RTNAME(ProgramStart)(argc, argv, {});
+RTNAME(ProgramStart)(argc, argv, {}, {});
   }
 
   std::string GetPaddedStr(const char *text, std::size_t len) const {
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -220,7 +220,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), parsing.allCooked(), "",
-  kindMap);
+  kindMap, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
   std::error_code ec;
Index: flang/test/Runtime/no-cpp-dep.c
===
--- flang/test/Runtime/no-cpp-dep.c
+++ flang/test/Runtime/no-cpp-dep.c
@@ -16,18 +16,20 @@
 we're testing. We can't include any headers directly since they likely contain
 C++ code that would explode here.
 */
+struct EnvironmentDefaultList;
 struct Descriptor;
 
 double RTNAME(CpuTime)();
 
-void RTNAME(ProgramStart)(int, const char *[], const char *[]);
+void RTNAME(ProgramStart)(int, const char *[], const char *[],
+  const struct EnvironmentDefaultList *);
 int32_t RTNAME(ArgumentCount)();
 int32_t RTNAME(GetCommandArgument)(int32_t, const struct Descriptor *,
 const struct Descriptor *, const struct Descriptor *);
 
 int main() {
   double x = RTNAME(CpuTime)();
-  RTNAME(ProgramStar

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic marked 2 inline comments as done.
jpenix-quic added inline comments.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:52
+options::OPT_fno_automatic,
+options::OPT_fconvert_EQ});
 }

awarzynski wrote:
> jpenix-quic wrote:
> > Reading through https://reviews.llvm.org/D95460 again, I'm not sure this is 
> > the appropriate place to add . I am marking this as a TODO that I will 
> > revisit with the other feedback!
> You can use `AddOtherOptions` instead. `AddFortranDialectOptions` is more 
> about language options. Is this a language option? It's a bit of a 
> borderline. Feel free to add another hook too.
> 
> Btw, the reformatting is an unrelated change. Could you submit in a separate 
> patch? Thanks!
I went the `AddOtherOptions` route for now. Apologies for the unrelated change 
and thank you for checking over this!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic updated this revision to Diff 447838.
jpenix-quic added a comment.

Added changes to address feedback about missing braces and where I am adding 
options::OPT_fconvert_EQ (and removing the extra formatting change). Also 
removes an unnecessary include I had mistakenly added.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/Convert.h
  flang/include/flang/Runtime/convert.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/Convert.cpp
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/ExternalIOTest.cpp

Index: flang/unittests/Runtime/ExternalIOTest.cpp
===
--- flang/unittests/Runtime/ExternalIOTest.cpp
+++ flang/unittests/Runtime/ExternalIOTest.cpp
@@ -149,6 +149,67 @@
   << "EndIoStatement() for Close";
 }
 
+TEST(ExternalIOTests, TestDirectUnformattedSwappedConvert) {
+  // OPEN(NEWUNIT=unit,ACCESS='DIRECT',ACTION='READWRITE',&
+  //   FORM='UNFORMATTED',RECL=8,STATUS='SCRATCH',CONVERT='NATIVE')
+  auto *io{IONAME(BeginOpenNewUnit)(__FILE__, __LINE__)};
+  ASSERT_TRUE(IONAME(SetAccess)(io, "DIRECT", 6)) << "SetAccess(DIRECT)";
+  ASSERT_TRUE(IONAME(SetAction)(io, "READWRITE", 9)) << "SetAction(READWRITE)";
+  ASSERT_TRUE(IONAME(SetForm)(io, "UNFORMATTED", 11)) << "SetForm(UNFORMATTED)";
+  ASSERT_TRUE(IONAME(SetConvert)(io, "NATIVE", 6)) << "SetConvert(NATIVE)";
+
+  std::int64_t buffer;
+  static constexpr std::size_t recl{sizeof buffer};
+  ASSERT_TRUE(IONAME(SetRecl)(io, recl)) << "SetRecl()";
+  ASSERT_TRUE(IONAME(SetStatus)(io, "SCRATCH", 7)) << "SetStatus(SCRATCH)";
+
+  int unit{-1};
+  ASSERT_TRUE(IONAME(GetNewUnit)(io, unit)) << "GetNewUnit()";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for OpenNewUnit";
+
+  static constexpr int records{10};
+  for (int j{1}; j <= records; ++j) {
+// WRITE(UNIT=unit,REC=j) j
+io = IONAME(BeginUnformattedOutput)(unit, __FILE__, __LINE__);
+ASSERT_TRUE(IONAME(SetRec)(io, j)) << "SetRec(" << j << ')';
+buffer = j;
+ASSERT_TRUE(IONAME(OutputUnformattedBlock)(
+io, reinterpret_cast(&buffer), recl, recl))
+<< "OutputUnformattedBlock()";
+ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+<< "EndIoStatement() for OutputUnformattedBlock";
+  }
+
+  // Set unformatted conversion to SWAP
+  RTNAME(ConvertOption)(4);
+  // OPEN(UNIT=unit,STATUS='OLD')
+  io = IONAME(BeginOpenUnit)(unit, __FILE__, __LINE__);
+  ASSERT_TRUE(IONAME(SetStatus)(io, "OLD", 3)) << "SetStatus(OLD)";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for OpenUnit";
+
+  for (int j{records}; j >= 1; --j) {
+// READ(UNIT=unit,REC=j) n
+io = IONAME(BeginUnformattedInput)(unit, __FILE__, __LINE__);
+ASSERT_TRUE(IONAME(SetRec)(io, j)) << "SetRec(" << j << ')';
+ASSERT_TRUE(IONAME(InputUnformattedBlock)(
+io, reinterpret_cast(&buffer), recl, recl))
+<< "InputUnformattedBlock()";
+ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+<< "EndIoStatement() for InputUnformattedBlock";
+ASSERT_EQ(buffer >> 56, j)
+<< "Read back " << (buffer >> 56) << " from direct unformatted record "
+<< j << ", expected " << j << '\n';
+  }
+
+  // CLOSE(UNIT=unit,STATUS='DELETE')
+  io = IONAME(BeginClose)(unit, __FILE__, __LINE__);
+  ASSERT_TRUE(IONAME(SetStatus)(io, "DELETE", 6)) << "SetStatus(DELETE)";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for Close";
+}
+
 TEST(ExternalIOTests, TestSequentialFixedUnformatted) {
   // OPEN(NEWUNIT=unit,ACCESS='SEQUENTIAL',ACTION='READWRITE',&
   //   FORM='UNFORMATTED',RECL=8,STATUS='SCRATCH')
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -33,6 +33,7 @@
 #include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "flang/Parser/unparse.h"
+#include "flang/Runtime/convert.h"
 #include "flang/Semantics/expression.h"
 #include "flang/Semantics/runtime-type-info.h"
 #include "flang/Semantics/semantics.h"
@@ -219,7 +220,7 @@
   auto burnside = Fortran::lower::LoweringBridge::create(
   ctx, defKinds, semanticsContext.intrinsics(),
   semanticsContext.targetCharacteristics(), pars

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4897
 def ffixed_line_length_VALUE : Joined<["-"], "ffixed-line-length-">, 
Group, Alias;
+def fconvert_EQ : Joined<["-"], "fconvert=">, Group,
+  HelpText<"Set endian conversion of data for unformatted files">;

peixin wrote:
> Why do you move it here? Maybe it is not implemented now, clang may need this 
> option eventually. @MaskRay 
I was using the fixed line length options as a reference for how to handle 
this--based on the discussion in the review here 
(https://reviews.llvm.org/D95460) about forwarding options to gfortran, I was 
thinking that it would also be safe to handle fconvert similarly, but I'm not 
100% sure and definitely might be misunderstanding something!



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:52
+options::OPT_fno_automatic,
+options::OPT_fconvert_EQ});
 }

Reading through https://reviews.llvm.org/D95460 again, I'm not sure this is the 
appropriate place to add . I am marking this as a TODO that I will revisit with 
the other feedback!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.

Thank you for taking a look at this and thank you for the feedback!

I'm not sure if I am entirely following/I think I am confusing myself on some 
of the specifics. My understanding of your suggestion is that, rather than add 
and create a call to a fconvert-specific runtime function (`ConvertOption`) I 
should look towards implementing and calling a runtime function/API that 
(conceptually) looks something like `SetRuntimeDefaults(FORT_CONVERT="SWAP", 
[...])` to set whatever was specified on the command line.

I think where I'm getting confused is when you mention default environment 
variable settings or translating the command-line options into the equivalent 
environment settings and passing them as defaults: is the idea to set the 
actual (in this case) `FORT_CONVERT` environment variable if it hasn't already 
been defined in the environment, with the goal of letting the existing 
`FORT_CONVERT` handling deal with everything? Or, is directly setting the 
`executionEnvironment.conversion` value like I tried to do in `ConvertOption` 
ok, but I need to rethink the `ConvertOption` API itself? If the goal is to set 
`FORT_CONVERT`, I'm getting a bit hung up on `FORT_CONVERT` currently being 
handled as part of the "hardcoded" main in Fort_main.c.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic added a comment.

I do have a few related lingering questions as well:

1. While this implementation mimics gfortran's behavior, there are still 
differences with other compilers (please see the comment here: 
https://github.com/llvm/llvm-project/issues/55961#issuecomment-1175677659). Is 
there a good place to document this (if worthwhile)?
2. There are a few differences in how gfortran and this implementation 
prioritize the convert command line option, environment variable, and CONVERT= 
specifier (please see comment here 
https://github.com/llvm/llvm-project/issues/55961#issuecomment-1172547132). 
Would this be worthwhile to change to match gfortran as well? If not, is there 
maybe a good place to document this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130513/new/

https://reviews.llvm.org/D130513

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jonathon Penix via Phabricator via cfe-commits]

jpenix-quic created this revision.
jpenix-quic added reviewers: schweitz, klausler, peixin, awarzynski.
jpenix-quic added a project: Flang.
Herald added subscribers: mehdi_amini, jdoerfert, mgorny.
Herald added a reviewer: sscalpone.
Herald added a project: All.
jpenix-quic requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

This follows gfortran's approach of generating a runtime call to set
the conversion state for the entire program and takes effect only if
the fconvert option is used on the main program (as the runtime call
is inserted into _QQmain).

Resolves issue: https://github.com/llvm/llvm-project/issues/55961


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Lower/Bridge.h
  flang/include/flang/Optimizer/Builder/Runtime/Convert.h
  flang/include/flang/Runtime/convert.h
  flang/include/flang/Runtime/main.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Lower/Bridge.cpp
  flang/lib/Optimizer/Builder/CMakeLists.txt
  flang/lib/Optimizer/Builder/Runtime/Convert.cpp
  flang/runtime/environment.cpp
  flang/runtime/environment.h
  flang/runtime/main.cpp
  flang/test/Driver/convert.f90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/tools/bbc/bbc.cpp
  flang/unittests/Runtime/ExternalIOTest.cpp

Index: flang/unittests/Runtime/ExternalIOTest.cpp
===
--- flang/unittests/Runtime/ExternalIOTest.cpp
+++ flang/unittests/Runtime/ExternalIOTest.cpp
@@ -149,6 +149,67 @@
   << "EndIoStatement() for Close";
 }
 
+TEST(ExternalIOTests, TestDirectUnformattedSwappedConvert) {
+  // OPEN(NEWUNIT=unit,ACCESS='DIRECT',ACTION='READWRITE',&
+  //   FORM='UNFORMATTED',RECL=8,STATUS='SCRATCH',CONVERT='NATIVE')
+  auto *io{IONAME(BeginOpenNewUnit)(__FILE__, __LINE__)};
+  ASSERT_TRUE(IONAME(SetAccess)(io, "DIRECT", 6)) << "SetAccess(DIRECT)";
+  ASSERT_TRUE(IONAME(SetAction)(io, "READWRITE", 9)) << "SetAction(READWRITE)";
+  ASSERT_TRUE(IONAME(SetForm)(io, "UNFORMATTED", 11)) << "SetForm(UNFORMATTED)";
+  ASSERT_TRUE(IONAME(SetConvert)(io, "NATIVE", 6)) << "SetConvert(NATIVE)";
+
+  std::int64_t buffer;
+  static constexpr std::size_t recl{sizeof buffer};
+  ASSERT_TRUE(IONAME(SetRecl)(io, recl)) << "SetRecl()";
+  ASSERT_TRUE(IONAME(SetStatus)(io, "SCRATCH", 7)) << "SetStatus(SCRATCH)";
+
+  int unit{-1};
+  ASSERT_TRUE(IONAME(GetNewUnit)(io, unit)) << "GetNewUnit()";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for OpenNewUnit";
+
+  static constexpr int records{10};
+  for (int j{1}; j <= records; ++j) {
+// WRITE(UNIT=unit,REC=j) j
+io = IONAME(BeginUnformattedOutput)(unit, __FILE__, __LINE__);
+ASSERT_TRUE(IONAME(SetRec)(io, j)) << "SetRec(" << j << ')';
+buffer = j;
+ASSERT_TRUE(IONAME(OutputUnformattedBlock)(
+io, reinterpret_cast(&buffer), recl, recl))
+<< "OutputUnformattedBlock()";
+ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+<< "EndIoStatement() for OutputUnformattedBlock";
+  }
+
+  // Set unformatted conversion to SWAP
+  RTNAME(ConvertOption)(4);
+  // OPEN(UNIT=unit,STATUS='OLD')
+  io = IONAME(BeginOpenUnit)(unit, __FILE__, __LINE__);
+  ASSERT_TRUE(IONAME(SetStatus)(io, "OLD", 3)) << "SetStatus(OLD)";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for OpenUnit";
+
+  for (int j{records}; j >= 1; --j) {
+// READ(UNIT=unit,REC=j) n
+io = IONAME(BeginUnformattedInput)(unit, __FILE__, __LINE__);
+ASSERT_TRUE(IONAME(SetRec)(io, j)) << "SetRec(" << j << ')';
+ASSERT_TRUE(IONAME(InputUnformattedBlock)(
+io, reinterpret_cast(&buffer), recl, recl))
+<< "InputUnformattedBlock()";
+ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+<< "EndIoStatement() for InputUnformattedBlock";
+ASSERT_EQ(buffer >> 56, j)
+<< "Read back " << (buffer >> 56) << " from direct unformatted record "
+<< j << ", expected " << j << '\n';
+  }
+
+  // CLOSE(UNIT=unit,STATUS='DELETE')
+  io = IONAME(BeginClose)(unit, __FILE__, __LINE__);
+  ASSERT_TRUE(IONAME(SetStatus)(io, "DELETE", 6)) << "SetStatus(DELETE)";
+  ASSERT_EQ(IONAME(EndIoStatement)(io), IostatOk)
+  << "EndIoStatement() for Close";
+}
+
 TEST(ExternalIOTests, TestSequentialFixedUnformatted) {
   // OPEN(NEWUNIT=unit,ACCESS='SEQUENTIAL',ACTION='READWRITE',&
   //   FORM='UNFORMATTED',RECL=8,STATUS='SCRATCH')
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -33,6 +33,7 @@
 #include "flang/Parser/parsing.h"
 #