This is an archive of the discontinued LLVM Phabricator instance.

Add LLVM_VERSION_SUFFIX to llvm-config.h.cmake
Needs ReviewPublic

Authored by kwryankrattiger on Dec 15 2021, 11:52 AM.

Details

Reviewers
chandlerc

Diff Detail

Event Timeline

kwryankrattiger requested review of this revision.Dec 15 2021, 11:52 AM

I am not sure what the test is failing on. It seems like a script error or something else unrelated to this change. I have successfully built llvm with this patch via the spack package manager on an AMD zen2 system with gcc @9.3.0 and used the resultant code to build mesa.

chuckatkins added a subscriber: chuckatkins.EditedDec 17 2021, 8:18 AM

The motivation for this patch btw is to allow a consumer to distinguish between release and development versions. Since the LLVM version numbers are incremented as soon as development of that version starts then there's not currently a way in the consuming C++ code to depend on the actual release. For example, in the Mesa llvmpipe driver the following can still fail when using a pre-release commit of llvm since setOverrideStackAlignment was added during the 13.0.0 development cycle and can't be guaranteed available until the 13.0.0 release:

extern "C" void
lp_set_module_stack_alignment_override(LLVMModuleRef MRef, unsigned align)
{
#if LLVM_VERSION_MAJOR >= 13
   llvm::Module *M = llvm::unwrap(MRef);
   M->setOverrideStackAlignment(align);
#endif
}

Using the LLVM_VERSION_SUFFIX to distinguish between release and development versions allows this to be changed to depend on the 13.0.0 release or newer via if (LLVM_VERSION = 13.0.0 release) or (LLVM_VERSION > 13.0.0):

extern "C" void
lp_set_module_stack_alignment_override(LLVMModuleRef MRef, unsigned align)
{
#if (LLVM_VERSION_MAJOR == 13 && LLVM_VERSION_MINOR == 0 && LLVM_VERSION_PATCH == 0 && !defined(LLVM_VERSION_SUFFIX)) || \
    (LLVM_VERSION_MAJOR >= 13 && LLVM_VERSION_MINOR >= 0 && LLVM_VERSION_PATCH > 0)
   llvm::Module *M = llvm::unwrap(MRef);
   M->setOverrideStackAlignment(align);
#endif
}

@chandlerc @mgorny Now that this is passing CI is there any opposition to merging this? How can we proceed?

@chandlerc @mgorny Is there anything else I need to do to move this patch forward?