[RFC] Split ConvertExpr.cpp

Subprojects Flang
1

I’ve been making some changes to flang/lib/Lower/ConvertExpr.cpp recently and I noticed it takes a very long time to compile. On a fast AArch64 machine, it usually takes 7m45s to compile and memory usage is over 6GB. This hampers development.

I want to try to split this file in 2. My idea is to move the ArrayExprLowering class to a new file, ConvertArrayExpr.cpp. It has 4008 lines, so ConvertExpr.cpp lines would drop from 7550 to 3542. Some static functions may also need to be moved or converted to non-static to be used by ArrayExprLowering. My hope is that this will reduce incremental build times by a substantial amount.

Before starting this, I would like to know if the community agrees, and if this change would be accepted, if it indeed reduces incremental build times. Thanks!

1 Like
2

I’m working on something similar: moving HLFIR transformational intrinsic lowering into a different file.

I suspect that build times will improve mostly from reducing the number of includes in the file I am working on, rather than the line count of the .cpp file, but I haven’t gotten far enough yet to confirm this.

+1 from me for your change.

1 Like
3

Are you building a Release or Debug configuration? Are you building with GCC or Clang?

I use an AMD Rome server for my Release builds using GCC 9.3.0 and things are very quick.

4

I’m building a Release configuration. In Debug configuration the time drops to about 5 minutes, on the same machine. But then the resulting compiler is slower to run the tests. I’m building with Clang 15.

I haven’t measured the time on x86_64 yet.

5

Splitting ConvertExpr.cpp was one of the goal of the lowering update. As such, ConvertExpr.cpp is not used anymore when lowering to HLFIR, and instead a series of smaller files is used (ConvertExprToHLFIR.cpp, ConvertCall.cpp, ConvertArrayConstructor.cpp, ConvertConstant.cpp).

The plan is to remove ConvertExpr.cpp when lowering to HLFIR is default and all LIT tests have been ported.

I am not against the split of ConvertExpr.cpp if it is big inconvenience to you, but I would recommend to spend too much time doing this. The priority for lowering in my opinion is to be able to switch to HLFIR by default to avoid work duplication.

6

Thanks for the clarification Jean.

In my above comment, I confused ConvertExpr.cpp and ConvertCall.cpp. What I said was not relevant to this discussion.

7

Thanks for the feedback.
If the plan is to remove ConvertExpr.cpp in the future, then I’m not sure if it’s worth to spend time splitting it.
Is there any rough estimate of when HLFIR will be ready to become the default? A few months, an year?