This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add flag for preventing the extension to 64 bits for the collapse loop counter
ClosedPublic

Authored by gtbercea on Dec 20 2018, 6:41 AM.

Details

Summary

Introduce a compiler flag for cases when the user knows that the collapsed loop counter can be safely represented using at most 32 bits. This will prevent the emission of expensive mathematical operations (such as the div operation) on the iteration variable using 64 bits where 32 bit operations are sufficient.

Diff Detail

Repository
rC Clang

Event Timeline

gtbercea created this revision.Dec 20 2018, 6:41 AM
ABataev added inline comments.Dec 20 2018, 6:52 AM
docs/OpenMPSupport.rst
119

-fopenmp-optimistic-collapse

include/clang/Basic/LangOptions.def
210

Fix the description and the option in accordance with the option name

include/clang/Driver/Options.td
1577

Missed -fno... option

lib/Driver/ToolChains/Clang.cpp
4429

You need to change the processing taking into account -fno... option.

lib/Frontend/CompilerInvocation.cpp
2848

Also, need to check for -fno... option

kkwli0 added a subscriber: kkwli0.Dec 20 2018, 7:15 AM
kkwli0 added inline comments.
docs/OpenMPSupport.rst
119

How about -fopenmp-use-32bit-collapse-parameter?

gtbercea updated this revision to Diff 179076.Dec 20 2018, 7:24 AM
gtbercea marked 4 inline comments as done.
gtbercea edited the summary of this revision. (Show Details)
  • Address comments.
gtbercea added inline comments.Dec 20 2018, 7:25 AM
include/clang/Basic/LangOptions.def
210

The description accurately describes what Sema is doing. We allow an extension to up to 32 bits but no further.

ABataev added inline comments.Dec 20 2018, 7:28 AM
lib/Driver/ToolChains/Clang.cpp
4429

fno_openmp_optimistic_collapse is not a frontend option. better to check for

if (Args.hasFlag(options::OPT_fopenmp_optimistic_collapse,
                   options::OPT_fno_openmp_optimistic_collapse,
                   /*Default=*/false))
  CmdArgs.push_back("-fopenmp-optimistic-collapse");

here rather than in the frontend. In the frontend, you can just check for the presence of the fopenmp_optimistic_collapse flag

gtbercea updated this revision to Diff 179078.Dec 20 2018, 7:40 AM
  • Address comments.
gtbercea marked an inline comment as done.Dec 20 2018, 7:40 AM
This revision is now accepted and ready to land.Dec 20 2018, 7:41 AM
gtbercea added inline comments.Dec 20 2018, 7:43 AM
docs/OpenMPSupport.rst
119

Alexey suggested we have no numbers in the flag name so we tried to find something that avoids that. I'm happy to give it a different name than optimistic collapse.

hfinkel added inline comments.
docs/OpenMPSupport.rst
120

Can you please clarify here what happens when the loop induction variables are already 64 bits. If any of them are already 64 bits, then we still use 64 bits overall?

gtbercea marked an inline comment as done.Jan 9 2019, 12:15 PM
gtbercea added inline comments.
docs/OpenMPSupport.rst
120

This flag is for the user to guarantee that the total size of the collapsed loops can be represented using at most 32 bits (regardless of the actual width of the individual loop induction variables).

This revision was automatically updated to reflect the committed changes.