This is an archive of the discontinued LLVM Phabricator instance.

[flang] Add builder/worker for debug build with reverse iteration
ClosedPublic

Authored by DavidSpickett on Aug 7 2023, 4:08 AM.

Details

Summary

For https://discourse.llvm.org/t/reverse-iteration-bots/72224.

This does not warrant a whole new builder, so I'm adding reverse
iteration to the debug configuration.

I thought of a few other places but:

  • The release builder tests what we will later ship, so I don't want to change the configuration.
  • Release with asserts is a very common developer configuration so it helps to know if problems occur with that only.
  • The rest are build types, out of tree, shared libs, etc. which are niche enough I don't want reversed iterator problems polluting them.

Initially I'm not removing the existing worker, so that the two
can run in parallel with the reverse iterator bot on silent.

Once it's been setup and is stable, it'll move to normal and
I'll remove the old worker and builder.

Event Timeline

DavidSpickett created this revision.Aug 7 2023, 4:08 AM
Herald added a project: Restricted Project. · View Herald Transcript
DavidSpickett requested review of this revision.Aug 7 2023, 4:08 AM

Thanks David for adding this builder. Re debug builder do you plan to remove the flang-aarch64-debug after the new builder is up and running. I guess if we add a new builder with DLLVM_REVERSE_ITERATION=ON we ll loose the ccache advantage given many files will get built twice for the same set of changes.

buildbot/osuosl/master/config/builders.py
2072

Debug builds sometime face bottleneck on memory. Should we consider adding -DLLVM_PARALLEL_LINK_JOBS to a reasonable number?

Add the "rev_it" tag used by the existing reverse iteration bot.

Never mind got the answer from the summary.

DavidSpickett marked an inline comment as done.
DavidSpickett added a subscriber: MaskRay.

Re debug builder do you plan to remove the flang-aarch64-debug after the new builder is up and running.

I guess if we add a new builder with DLLVM_REVERSE_ITERATION=ON we ll loose the ccache advantage given many files will get built twice for the same set of changes.

Yes, but that wasn't my motivation in (eventually) replacing the debug builder. The motivation is to keep the number of bots down and not add too many specialty configurations that report mostly the same results.
(and I'm renaming just so people don't have to look into the cmake)

In this case, I think the problems caused by debug mode and those with reverse iteration will likely be easy to tell apart.

Although, @MaskRay can you confirm that there's no conflict between reverse iteration and debug builds? It's still useful coverage as I understand it.

buildbot/osuosl/master/config/builders.py
2072

They do but initially I'll run the worker on a different machine where memory isn't a concern. In the end it will replace the existing worker, which has not had any memory problems recently on its current host.

In general only in favour of adding these sort of machine specific details if there's no other way to handle it.

DavidSpickett marked an inline comment as done.Aug 7 2023, 4:42 AM

In case it's useful, this is what Linaro runs currently for flang:

clang-aarch64-full-2stage       
clang-arm64-windows-msvc        
clang-arm64-windows-msvc-2stage 
clang-aarch64-sve-vla           
clang-aarch64-sve-vla-2stage    
clang-aarch64-sve-vls           
clang-aarch64-sve-vls-2stage    
flang-aarch64-dylib             
flang-aarch64-sharedlibs        
flang-aarch64-out-of-tree       
flang-aarch64-release           
flang-aarch64-debug             
flang-aarch64-latest-clang      
flang-aarch64-rel-assert        
flang-aarch64-latest-gcc

To give you an idea why I'd like to replace an existing configuration rather than adding a new one on top of all that. Feel free to disagree of course, or suggest another home for reverse iteration.

In case it's useful, this is what Linaro runs currently for flang:

clang-aarch64-full-2stage       
clang-arm64-windows-msvc        
clang-arm64-windows-msvc-2stage 
clang-aarch64-sve-vla           
clang-aarch64-sve-vla-2stage    
clang-aarch64-sve-vls           
clang-aarch64-sve-vls-2stage    
flang-aarch64-dylib             
flang-aarch64-sharedlibs        
flang-aarch64-out-of-tree       
flang-aarch64-release           
flang-aarch64-debug             
flang-aarch64-latest-clang      
flang-aarch64-rel-assert        
flang-aarch64-latest-gcc

To give you an idea why I'd like to replace an existing configuration rather than adding a new one on top of all that. Feel free to disagree of course, or suggest another home for reverse iteration.

I believe this list should slowly shrink over time but given flang is still not mature enough we might not see that happening anytime soon.

Very true.

And if having debug and reverse iteration on the same worker did prove to be confusing in practice, I'd of course split them up.

Thanks for adding a builder!

Re debug builder do you plan to remove the flang-aarch64-debug after the new builder is up and running.

I guess if we add a new builder with DLLVM_REVERSE_ITERATION=ON we ll loose the ccache advantage given many files will get built twice for the same set of changes.

Yes, but that wasn't my motivation in (eventually) replacing the debug builder. The motivation is to keep the number of bots down and not add too many specialty configurations that report mostly the same results.
(and I'm renaming just so people don't have to look into the cmake)

In this case, I think the problems caused by debug mode and those with reverse iteration will likely be easy to tell apart.

Although, @MaskRay can you confirm that there's no conflict between reverse iteration and debug builds? It's still useful coverage as I understand it.

LLVM_ENABLE_REVERSE_ITERATION=on (currently, LLVM_REVERSE_ITERATION=on achieves the same effect) is orthogonal to CMAKE_BUILD_TYPE=Debug, LLVM_ENABLE_ASSERTIONS=on.

To give you an idea why I'd like to replace an existing configuration rather than adding a new one on top of all that. Feel free to disagree of course, or suggest another home for reverse iteration.

And if having debug and reverse iteration on the same worker did prove to be confusing in practice, I'd of course split them up.

My gut feeling is that combining some configurations will make more effective uses of the resources, but you maintainers make the call:)

This revision is now accepted and ready to land.Aug 10 2023, 10:09 AM

@omjavaid / @luporl / @sscalpone If you have any further comments please make them before Tuesday next week. If not, I'll go ahead with this as is.

MaskRay accepted this revision.Aug 11 2023, 8:43 AM