[NVPTX] Allow degenerated `nvvm.annotations`

[jdoerfert]

Summary

As part of D120129 we started to assert if an nvvm.annotation had a
nullptr as a first component. That is however happening with OpenMP
offload and older CUDA versions, e.g., CUDA 11.0.2, while newer ones,
e.g., 11.4.0 work fine. There is no need to require a non-null value
so let's keep things working for old codes.

[kovdan01]

Thanks @jdoerfert! I was going to implement a fix for that, but have not managed to do it yet. See a couple of inline comments below.

llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

4273

In D120129, you added a comment containing the following piece of IR:

!nvvm.annotations = !{!8, !9, !8, !10, !10, !10, !10, !11, !11, !10}

!8 = !{null, !"align", i32 8}
!9 = !{null, !"align", i32 8, !"align", i32 65544, !"align", i32 131080}
!10 = !{null, !"align", i32 16}
!11 = !{null, !"align", i32 16, !"align", i32 65552, !"align", i32 131088}

According to that, one piece of metadata can contain more than 3 operands. I was unable to find a piece of code where we create such piece of metadata. I even was ubable to find code that adds align info to nvvm.annotations. All that I have found – NVPTXTargetCodeGenInfo::addNVVMMetadata and similar/duplicating code, that creates a piece of metadata consisting of 3 operands. So, I can’t state for sure without seeing code responsible for that, but given your IR above, a question arises: shouldn’t we change this assert?

llvm/test/CodeGen/NVPTX/param-vectorize-device.ll

803

I think that we should add a comment explaining what is the purpose of these lines. It might be unclear out of the context of the revision. See ; Section X - checking that... comments above.

[kovdan01]

llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

4268–4296

During investigation, discovered a nice function bool isKernelFunction(const Function &); in NVPTXUtilities. It does the same thing (with going inside metadata) but correctly in its implementation. So I suppose that we can replace the whole ifndef block with a single asserion:

assert(!isKernelFunction(*F));

Also, if we use an already existing function instead of reinventing a wheel, looks like there is no need in altering tests.

[jdoerfert]

D120129 needs to be reverted and fixed properly.

llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

Align NVPTXTargetLowering::getFunctionParamOptimizedAlign(
 
   // If a function has linkage different from internal or private, we
   // must use default ABI alignment as external users rely on it.
   switch (F->getLinkage()) {
   case GlobalValue::InternalLinkage:
   case GlobalValue::PrivateLinkage: {
     // Check that if a function has internal or private linkage
     // it is not a kernel.
 #ifndef NDEBUG
     const NamedMDNode *NMDN =
         F->getParent()->getNamedMetadata("nvvm.annotations");
     if (NMDN) {
       for (const MDNode *MDN : NMDN->operands()) {
         assert(MDN->getNumOperands() == 3);
 
         const Metadata *MD0 = MDN->getOperand(0).get();
+        // Allow degenerated annotations created by OpenMP offloading with old
+        // CUDA versions.
+        if (!MD0)
+          continue;
+
         const auto *MDV0 = cast<ConstantAsMetadata>(MD0)->getValue();
         const auto *MDFn = cast<Function>(MDV0);
         if (MDFn != F)
           continue;
 
         const Metadata *MD1 = MDN->getOperand(1).get();
         const MDString *MDStr = cast<MDString>(MD1);
         if (MDStr->getString() != "kernel")
           continue;
 
         const Metadata *MD2 = MDN->getOperand(2).get();
         const auto *MDV2 = cast<ConstantAsMetadata>(MD2)->getValue();
         assert(!cast<ConstantInt>(MDV2)->isZero());
       }
     }
 #endif
     return Align(std::max(uint64_t(16), ABITypeAlign));
   }
   default:
     return Align(ABITypeAlign);
   }
 }
 
 /// isLegalAddressingMode - Return true if the addressing mode represented

llvm/test/CodeGen/NVPTX/param-vectorize-device.ll

define external fastcc [4 x i32] @callee_St4x4_external(%struct.St4x4* nocapture noundef readonly byval(%struct.St4x4) align 4 %in) {
   %arrayidx.3 = getelementptr inbounds %struct.St4x4, %struct.St4x4* %in, i64 0, i32 0, i64 3
   %4 = load i32, i32* %arrayidx.3, align 4
   %5 = insertvalue [4 x i32] poison, i32 %1, 0
   %6 = insertvalue [4 x i32] %5, i32 %2, 1
   %7 = insertvalue [4 x i32] %6, i32 %3, 2
   %oldret = insertvalue [4 x i32] %7, i32 %4, 3
   ret [4 x i32] %oldret
 }
+
+!nvvm.annotations = !{!0}
+
+!0 = !{null, !"foobar", i32 0}