[Clang][CodeGen] Fix bad codegen when building Clang with latest MSVC

[aganea]

Before this PR, when using the latest MSVC Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33813 for x64 one of the Clang unit test used to fail: CodeGenObjC/gnustep2-direct-method.m, see full failure log here.

This seems to have been introduced by c9e5af3 however further inspection shows that commit only triggers a bug in the MSVC compiler.

It seems that the symptom is bad alignement generated in one of the load instructions:

huge alignment values are unsupported
  %2 = load i64, ptr %1, align 9223372036854775808

When Builder.CreateLoad is called here, somehow this call to Addr.getAlignment().getAsAlign() returns a bad alignement. The problem occurs at the highlighted line in the screenshot (sub bh,cl):

The code line on the right is translated to the assembly on the left. llvm::count_zero returns a proper value (as seen in rcx), however sub bh, cl uses a bad constant in bh (it is not 63 as expected). I think the optimizer meant to use dil not bh. A few lines below it does mov byte ptr [rsp + 40h], dil. If after sub is executed I manually set 6 in rdi, as it should have been, the test passes.

This happens only in optimized builds. I used llvm\utils\release\build_llvm_release.bat --version 19.0.0 --x64 --skip-checkout --local-python to build a LLVM installer.

Bug reported here: https://developercommunity.visualstudio.com/t/Bad-code-generation-when-building-LLVM-w/10719589?port=1025&fsid=e572244a-cde7-4d75-a73d-9b8cd94204dd

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Alexandre Ganea (aganea)

Changes

Before this PR, when using the latest MSVC Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33813 for x64 one of the Clang unit test used to fail: CodeGenObjC/gnustep2-direct-method.m, see full failure log here.

This seems to have been introduced by c9e5af3 however further inspection shows that commit only triggers a bug in the MSVC compiler.

It seems that the symptom is bad alignement generated in one of the load instructions:

huge alignment values are unsupported
  %2 = load i64, ptr %1, align 9223372036854775808

When Builder.CreateLoad is called here, somehow this call to Addr.getAlignment().getAsAlign() returns a bad alignement. The problem occurs at the highlighted line in the screenshot (sub bh,cl):

The code line on the right is translated to the assembly on the right. llvm::count_zero returns a proper value (as seen in rcx), however sub bh, cl uses a bad constant in bh (it is not 63 as expected). I think the optimizer meant to use dil not bh. A few lines below it does mov byte ptr [rsp + 40h], dil. If after sub is executed I manually set 6 in rdi, as it should have been, the test passes.

I'll fix a bug with Microsoft will cross post it here.

---

Full diff: https://github.com/llvm/llvm-project/pull/102681.diff

1 Files Affected:

• (modified) clang/lib/CodeGen/CGObjCGNU.cpp (+4-4)

diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 948b10954ebbe..56a3ed1b87b35 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2092,10 +2092,10 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
         auto *classStart =
             llvm::StructType::get(PtrTy, PtrTy, PtrTy, LongTy, LongTy);
         auto &astContext = CGM.getContext();
-        auto flags = Builder.CreateLoad(
-            Address{Builder.CreateStructGEP(classStart, selfValue, 4), LongTy,
-                    CharUnits::fromQuantity(
-                        astContext.getTypeAlign(astContext.UnsignedLongTy))});
+        llvm::Value *Val = Builder.CreateStructGEP(classStart, selfValue, 4);
+        auto Align = CharUnits::fromQuantity(
+            astContext.getTypeAlign(astContext.UnsignedLongTy));
+        auto flags = Builder.CreateLoad(Address{Val, LongTy, Align});
         auto isInitialized =
             Builder.CreateAnd(flags, ClassFlags::ClassFlagInitialized);
         llvm::BasicBlock *notInitializedBlock =

[compnerd]

Might be nice to clean this up in the future when Microsoft releases a fix.

[davidchisnall]

The new version is probably slightly more readable than mine, but I don’t really understand how it would affect codegen with UT C. No objections.

[aganea]

Might be nice to clean this up in the future when Microsoft releases a fix.

I can add a comment to that effect.