Skip to content

Conversation

RiverDave
Copy link
Collaborator

@RiverDave RiverDave commented Aug 28, 2025

I found a blocker when working on PTX intrinsics.

One of the problems we have whenever we want to emit generic builtins with the current infrastructure (IIT Descriptors) is that it doesn't allow us to determine signedness in scalar types. When we emit these builtins we seem to type check param/return types:
if (argType != intrinsicType.getInput(i)). In our current approach to decode a type from IIT we force signedness:

static mlir::Type
decodeFixedType(ArrayRef<llvm::Intrinsic::IITDescriptor> &infos,
                mlir::MLIRContext *context) {
  using namespace llvm::Intrinsic;

  IITDescriptor descriptor = infos.front();
  infos = infos.slice(1);

  switch (descriptor.Kind) {
...
  case IITDescriptor::Integer:
    return IntType::get(context, descriptor.Integer_Width, /*isSigned=*/true);

I was finding cases where the function definition would specify x parameter as unsigned but when calling this function with the right type, it would trigger an NYI statement we had encoded(This is not case for OG as by design, LLVMIR does not contain that information):

if (argType != intrinsicType.getInput(i))
        llvm_unreachable("NYI");

Example issue:

  def __nvvm_bar_warp_sync : NVPTXBuiltinPTX<"void(unsigned int)", PTX60>;
__device__ void nvvm_sync(unsigned int mask) {
    __nvvm_bar_warp_sync(mask);  // expects unsigned, but IIT returns signed (wrong assumption)
}

To solve this, I've used a very naive approach, we know that front-end contains all signedness information we need. I've used AST information (I've specifically extracted the Function Decl) to determine whether if we should enforce signedness or not based on the definition it was declared with.

I've also added the respective CUDA test that was being affected by this issue. along with the implementing the rest of the NYI block related to this.

@RiverDave RiverDave changed the title [CIR][CIRGen] Fix intrinsic type signedness using frontend AST information [CIR][CIRGen] Fix builtin type signedness using frontend AST information Aug 28, 2025
@RiverDave RiverDave changed the title [CIR][CIRGen] Fix builtin type signedness using frontend AST information [CIR][CIRGen] Fix builtin type signedness using AST information Aug 28, 2025
@RiverDave RiverDave changed the title [CIR][CIRGen] Fix builtin type signedness using AST information [CIR][CIRGen] Fix builtin IIT type signedness using AST information Aug 28, 2025
@RiverDave RiverDave changed the title [CIR][CIRGen] Fix builtin IIT type signedness using AST information [CIR][CIRGen] Fix builtin IIT Integer signedness using AST information Aug 29, 2025
@RiverDave RiverDave force-pushed the dave/fix-iit-types branch 3 times, most recently from 8699d72 to 6e489c8 Compare September 3, 2025 04:13
Copy link

github-actions bot commented Sep 3, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, LGTM once a few final nits get address - thanks for your patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants