Fix build with LLVM 12 #171

zboszor · 2021-02-25T08:15:08Z

Signed-off-by: Zoltán Böszörményi [email protected]

Signed-off-by: Zoltán Böszörményi <[email protected]>

pszymich

We avoid adding multiple #if LLVM_VERSION_MAJOR blocks by moving them to methods in LLVMWrapper in IGCLLVM scope, I've posted some comments on this as well as other minor changes.

IGC/Compiler/Optimizer/Scalarizer.cpp

IGC/Compiler/ConvertMSAAPayloadTo16Bit.cpp

IGC/Compiler/Legalizer/InstPromoter.cpp

IGC/Compiler/Optimizer/OpenCLPasses/AddressSpaceAliasAnalysis/AddressSpaceAliasAnalysis.cpp

pszymich · 2021-02-25T14:24:09Z

IGC/AdaptorOCL/SPIRV/SPIRVReader.cpp

+#if LLVM_VERSION_MAJOR >= 12
+            I->setDebugLoc(DILocation::get(scope->getContext(), Line->getLine(), Line->getColumn(),
+#else
            I->setDebugLoc(DebugLoc::get(Line->getLine(), Line->getColumn(),
+#endif


There are multiple DILocation::get() and DebugLoc::get() uses in your commit. Each one is wrapped with the #if LLVM_VERSION... guard; we would like to avoid these.
This is a candidate for a new IGCLLVM::getDILocation() wrapper method in WrapperLLVM module.
Have you verified that DILocation::get() works only since LLVM 12? If it is a case where DebugLoc::get() is deprecated since LLVM 12, but DILocation::get() have worked previously we would like to minimize the LLVM version in the #if guard.

What's the earliest version of LLVM that IGC supports?

Nevermind. I reviewed git changes back to about LLVM 6 or 7 and no substantial changes went into class DILocation or class MDNode, so DILocation::get() should work for those versions. I couldn't confirm it by actually building IGC because I don't have any machine using such an old LLVM version.

pszymich · 2021-02-25T14:26:48Z

IGC/Compiler/CISACodeGen/ShaderCodeGen.cpp

@@ -589,12 +589,14 @@ static void AddLegalizationPasses(CodeGenContext& ctx, IGCPassManager& mpm, PSSi
    {
        mpm.add(createPruneUnusedArgumentsPass());

+#if LLVM_VERSION_MAJOR < 12


Why are these passes cut?

They were removed from LLVM 12.
See the removal commits 599955eb56ebad50c12422cb6194a2da770902a0 and 486ed885339d70cd71ee55567282a43cce28d763 in LLVM.

IGC/Compiler/CISACodeGen/AdvMemOpt.cpp

IGC/Compiler/Optimizer/OpenCLPasses/AddressSpaceAliasAnalysis/AddressSpaceAliasAnalysis.cpp

IGC/AdaptorOCL/SPIRV/SPIRVReader.cpp

Signed-off-by: Zoltán Böszörményi <[email protected]>

pszymich · 2021-02-26T09:41:55Z

IGC/Compiler/Optimizer/OpenCLPasses/WIFuncs/WIFuncResolution.cpp

    if (DataType->isVectorTy())
    {
-        Size *= DataType->getVectorNumElements();
+        Size *= cast<IGCLLVM::FixedVectorType>(DataType)->getNumElements();
    }


This could be further simplified as such:

if (auto DataVT = dyn_cast<IGCLLVM::FixedVectorType>(DataType)) { Size *= DataVT->getNumElements(); }

but a case can be made it's less readable, so let's leave it as is.

pszymich

The changes LGTM.

We don't support merging pull-request directly from our public repository. I will mirror your pull-request internally for it to go through our verification processes and get back to you with results.

Thank you for your contribution Zoltán!

pszymich · 2021-03-11T11:14:53Z

The commit is being delayed by problems with internal verification. I will update this PR with necessary changes when I get it resolved.

Patches applied from Open PR#171, in order to build with llvm-12. intel/intel-graphics-compiler#171 Error logs: (1) | /build/tmp/work/corei7-64-poky-linux/intel-graphics-compiler/1.0.6410-r0/git/IGC/Compiler/CISACodeGen/VariableReuseAnalysis.hpp:83:56: error: 'unsigned int llvm::VectorType::getNumElements() const' is deprecated: Calling this function via a base VectorType is deprecated. Either call getElementCount() and handle the case where Scalable is true or cast to FixedVectorType. [-Werror=deprecated-declarations] | 83 | NumElts = VTy ? (short)VTy->getNumElements() : 1; Ref:llvm/llvm-project@867de15 (2) | /build/tmp/work/corei7-64-poky-linux/intel-graphics-compiler/1.0.6410-r0/git/IGC/common/igc_resourceDimTypes.h:69:23: error: 'const class llvm::Module' has no member named 'getTypeByName' | 69 | return module.getTypeByName(ResourceDimensionTypeName[resourceDimTypeId]); Ref: llvm/llvm-project@fe43168 Update copyright year in headers in IGC Compiler and some format updates. Signed-off-by: Naveen Saini <[email protected]> Signed-off-by: Anuj Mittal <[email protected]>

lin7sh · 2021-04-16T12:00:06Z

LLVM 12 officially released yesterday, any progress on this?

thiagomacieira · 2021-04-20T21:29:28Z

ping. What's taking time to even provide feedback?

pszymich · 2021-04-22T06:29:02Z

Hello,

@thiagomacieira, the delay with handling this PR was unfortunately caused by dropping LLVM11/12 upgrade priority internally, which I'm back to working on.

@zboszor, your pull-request is now merged, although I had to tweak a couple things, mainly the removed passes and merge conflicts, as well as some small additions like explicit casts to avoid error-on-warning buildbreaks. You can find the commit here: 4bc6b44
As all pull-requests in this repository, they have to go through our internal workflow. This is why the PR is not merged directly.
If you have any questions or concerns regarding this commit please contact me.

Thank you for your contribution!

foutrelis · 2021-05-27T15:03:55Z

Shouldn't this be reopened since it was backed out by 32944e6?

ArchangeGabriel · 2021-06-02T14:59:33Z

More generally, can we get insight on what is happening? The patch was commited and reverted twice…
4bc6b44
bcda9e4
1c28c74
32944e6

Patches applied from Open PR#171, in order to build with llvm-12. intel/intel-graphics-compiler#171 Error logs: (1) | /build/tmp/work/corei7-64-poky-linux/intel-graphics-compiler/1.0.6410-r0/git/IGC/Compiler/CISACodeGen/VariableReuseAnalysis.hpp:83:56: error: 'unsigned int llvm::VectorType::getNumElements() const' is deprecated: Calling this function via a base VectorType is deprecated. Either call getElementCount() and handle the case where Scalable is true or cast to FixedVectorType. [-Werror=deprecated-declarations] | 83 | NumElts = VTy ? (short)VTy->getNumElements() : 1; Ref:llvm/llvm-project@867de15 (2) | /build/tmp/work/corei7-64-poky-linux/intel-graphics-compiler/1.0.6410-r0/git/IGC/common/igc_resourceDimTypes.h:69:23: error: 'const class llvm::Module' has no member named 'getTypeByName' | 69 | return module.getTypeByName(ResourceDimensionTypeName[resourceDimTypeId]); Ref: llvm/llvm-project@fe43168 Update copyright year in headers in IGC Compiler and some format updates. Signed-off-by: Naveen Saini <[email protected]> Signed-off-by: Anuj Mittal <[email protected]>

zboszor force-pushed the llvm12-build-fixes branch from 2ae1f13 to 7ac1f3c Compare February 25, 2021 09:40

Fix build with LLVM 12

869cdb7

Signed-off-by: Zoltán Böszörményi <[email protected]>

zboszor force-pushed the llvm12-build-fixes branch from 7ac1f3c to 869cdb7 Compare February 25, 2021 10:07

pszymich requested changes Feb 25, 2021

View reviewed changes

Zoltán Böszörményi added 2 commits February 25, 2021 19:40

Review fixes for LLVM 12 phase 1

60136b4

Signed-off-by: Zoltán Böszörményi <[email protected]>

Review fixes for LLVM 12 phase 2

c6d3336

Signed-off-by: Zoltán Böszörményi <[email protected]>

pszymich reviewed Feb 26, 2021

View reviewed changes

ArchangeGabriel mentioned this pull request Apr 20, 2021

FTBFS with LLVM 11.1: "error: ‘class llvm::Type’ has no member named ‘getVectorElementType’; did you mean ‘getPointerElementType’" #178

Closed

pszymich closed this Apr 22, 2021

zboszor deleted the llvm12-build-fixes branch September 7, 2021 14:36

Fix build with LLVM 12 #171

Fix build with LLVM 12 #171

Uh oh!

Conversation

zboszor commented Feb 25, 2021

Uh oh!

pszymich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pszymich Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

zboszor Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

zboszor Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

pszymich Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

zboszor Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pszymich Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

pszymich left a comment

Choose a reason for hiding this comment

Uh oh!

pszymich commented Mar 11, 2021

Uh oh!

lin7sh commented Apr 16, 2021

Uh oh!

thiagomacieira commented Apr 20, 2021

Uh oh!

pszymich commented Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foutrelis commented May 27, 2021

Uh oh!

ArchangeGabriel commented Jun 2, 2021

Uh oh!

Uh oh!

pszymich commented Apr 22, 2021 •

edited

Loading