Avoid setting tbaa information on store of return type of call to inline assember

[schittir]

Summary

In 32bit mode, attaching TBAA metadata to the store following the call to inline assembler results in describing the wrong type by making a fake lvalue(i.e., whatever the inline assembler happens to leave in EAX:EDX.) Even if inline assembler somehow describes the correct type, setting TBAA information on return type of call to inline assembler is likely not correct, since TBAA rules need not apply to inline assembler.

[jeroen.dobbelaere]

Do you have a testcase ?

[schittir]

Add test case
Fix formatting

[schittir]

In D115320#3178682, @jeroen.dobbelaere wrote:

Do you have a testcase ?

Thank you for the review. Please take a look at the new diff.

[jeroen.dobbelaere]

When I try out the example on llvm-13, I get a 'omnipotent char' tbaa description. That should be ok in general. When I replace the 'float _Complex' with 'double', I do get the 'double' tbaa. That might be a better example for the testcase ?

[schittir]

Updated test per Jeroen's suggestion

[schittir]

In D115320#3182581, @jeroen.dobbelaere wrote:

When I try out the example on llvm-13, I get a 'omnipotent char' tbaa description. That should be ok in general. When I replace the 'float _Complex' with 'double', I do get the 'double' tbaa. That might be a better example for the testcase ?

Good point. I updated the test with this suggestion. Thank you.

[jeroen.dobbelaere]

clang/lib/CodeGen/CodeGenFunction.h

2511

Looking at how a 'null tbaa'is produced in other places (grep for 'TBAAAccessInfo()', I would just do a 'TBAAAccessInfo()' here and omit the new ''returnNullTBAA' method.

clang/test/CodeGen/avoidTBAAonASMstore.cpp

7

Will this also work in release mode ? Maybe abstract away the %2 and %1.

[schittir]

1. Removed returnNullTBAA() method
2. Add NO-TBAA check-prefix to the test

[jeroen.dobbelaere]

We're almost there. But the testcase is now not testing what it should be testing. Also the unused CHECK: must be adapted.

[schittir]

Changing test's CHECK lines

[schittir]

Fix test format

[schittir]

https://reviews.llvm.org/harbormaster/unit/view/1536685/
Test failure seems unrelated to this patch. Unable to reproduce locally.

[schittir]

Rebasing patch to see if the test failure will go away.

[jeroen.dobbelaere]

clang/test/CodeGen/avoidTBAAonASMstore.cpp

4

Shouldn't this be STORE-LINE-LABEL: ... ?

[schittir]

Change CHECK-LABEL to STORE-LINE-LABEL

[jeroen.dobbelaere]

LGTM

[DavidSpickett]

Hi, the new test is failing on our arm/aarch64 quick bots. These build only the respective backend so you might need to add a requires x86 target.

https://lab.llvm.org/buildbot/#/builders/171/builds/7189

/home/tcwg-buildbot/worker/clang-armv7-quick/llvm/clang/test/CodeGen/avoidTBAAonASMstore.cpp:5:3: error: MS-style inline assembly is not available: No available targets are compatible with triple "i386-unknown-linux-gnu"
  __asm { fnstcw word ptr[ControlWord]}
  ^
1 error generated.

[DavidSpickett]

I've gone ahead and added the REQUIRES myself.

[schittir]

In D115320#3195090, @DavidSpickett wrote:

I've gone ahead and added the REQUIRES myself.

Thank you, David!

clang/lib/CodeGen/CGStmt.cpp

void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   }
 
   // If this is a Microsoft-style asm blob, store the return registers (EAX:EDX)
   // to the return value slot. Only do this when returning in registers.
   if (isa<MSAsmStmt>(&S)) {
     const ABIArgInfo &RetAI = CurFnInfo->getReturnInfo();
     if (RetAI.isDirect() || RetAI.isExtend()) {
       // Make a fake lvalue for the return value slot.
-      LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+      LValue ReturnSlot = MakeAddrLValueWithoutTBAA(ReturnValue, FnRetTy);
       CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
           *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
           ResultRegDests, AsmString, S.getNumOutputs());
       SawAsmBlock = true;
     }
   }
 
   for (unsigned i = 0, e = S.getNumInputs(); i != e; i++) {

clang/lib/CodeGen/CodeGenFunction.h

public:
   }
 
   LValue MakeAddrLValue(llvm::Value *V, QualType T, CharUnits Alignment,
                         LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
     return LValue::MakeAddr(Address(V, Alignment), T, getContext(),
                             BaseInfo, TBAAInfo);
   }
 
+  LValue
+  MakeAddrLValueWithoutTBAA(Address Addr, QualType T,
+                            AlignmentSource Source = AlignmentSource::Type) {
+    return LValue::MakeAddr(Addr, T, getContext(), LValueBaseInfo(Source),
+                            TBAAAccessInfo());
+  }
+
   LValue MakeNaturalAlignPointeeAddrLValue(llvm::Value *V, QualType T);
   LValue MakeNaturalAlignAddrLValue(llvm::Value *V, QualType T);
 
   Address EmitLoadOfReference(LValue RefLVal,
                               LValueBaseInfo *PointeeBaseInfo = nullptr,
                               TBAAAccessInfo *PointeeTBAAInfo = nullptr);
   LValue EmitLoadOfReferenceLValue(LValue RefLVal);
   LValue EmitLoadOfReferenceLValue(Address RefAddr, QualType RefTy,

clang/test/CodeGen/avoidTBAAonASMstore.cpp

+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -disable-llvm-passes -fasm-blocks %s -emit-llvm -o - | FileCheck --check-prefix=STORE-LINE %s
+double foo(double z) {
+  // STORE-LINE-LABEL: define{{.*}} double @_Z3food
+  unsigned short ControlWord;
+  __asm { fnstcw word ptr[ControlWord]}
+  ;
+  // STORE-LINE: store i64 %{{.*}}, i64* %{{.*}},
+  // STORE-LINE-NOT: align 4, !tbaa
+  // STORE-LINE-SAME: align 4
+  return z;
+}