arnamoy10 added inline comments.

================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:254
+  if (args.hasArg(clang::driver::options::OPT_fdefault_real_8))
+    res.defaultKinds().set_defaultRealKind(8);
+  if (args.hasArg(clang::driver::options::OPT_fdefault_integer_8)) {
----------------
awarzynski wrote:
> From `gfortran` [[ 
> https://gcc.gnu.org/onlinedocs/gfortran/Fortran-Dialect-Options.html | 
> documentation ]]:
> 
> ```
> This option promotes the default width of DOUBLE PRECISION and double real 
> constants like 1.d0 to 16 bytes if possible.
> ```
> 
> So I believe that you are missing:
> ```
> res.defaultKinds().set_doublePrecisionKind(16);
> ```
I thought it is not necessary because `doublePrecisionKind` is automatically 
initialized to double the size of `defaultRealKind_` in [[ 
https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/flang/include/flang/Common/default-kinds.h#L51-L58
 | here ]]--> `int doublePrecisionKind_{2 * defaultRealKind_};`. 




================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:269-271
+    // For -fdefault-double-8 + -fdefault-real-8, only the size of
+    // DOUBLE PRECISION type changes, the size of default real type stays
+    // the same
----------------
awarzynski wrote:
> I think that it's more like:
> ```
> // -fdefault-real-8 alone promotes the width for DOUBLE PRECISION to 16 
> bytes. -fdefault-double-8 overwrites that and sets the width back to 8 bytes.
> ```
> Since this patch adds `gfortran` style semantics, it would be worth adding a 
> link to the docs somewhere at the top.
Sure, will do!


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

https://reviews.llvm.org/D96344

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

Reply via email to