Large constants in patchpoints

Currently llvm crashes on following code, run through llc:

declare void @llvm.experimental.stackmap(i64, i32, ...)

define void @foo() {
  tail call void (i64, i32, ...)* @llvm.experimental.stackmap(i64 0,
i32 0, i64 9223372036854775807)
  ret void
}

The issue is that 9223372036854775807 (decimal for 0x7fffffffffffffff)
is the "empty key" for an int64_t, and in
StackMaps::recordStackMapOpers we crash when we try to insert this as
a key into ConstPool. The this happens if we change the constant to
be the tombstone.

Two potential fixes I can think of:

1. have some special logic to remember the offsets for the tombstone
    and empty i64 constants. This can easily be tracked using two
    "Optional<int>" fields.

2. change ConstantPool to be use a std::map instead of a
    llvm::DenseMap as the map in the MapVector.

An aside: the same function has this check "((I->Offset +
(int64_t(1)<<31)) >> 32)" -- won't this cause a signed overflow (and
hence UB) if I->Offset is negative?

-- Sanjoy

Apologies, please ignore the aside:

An aside: the same function has this check "((I->Offset +
(int64_t(1)<<31)) >> 32)" -- won't this cause a signed overflow (and
hence UB) if I->Offset is negative?

The overflow won't be a *signed overflow*, so we're okay.

-- Sanjoy

We also could make the DenseMap just an uint64_t. In this case the empty key and the tombstone would be never encoded into the constant pool, because they can be encoded in the offset field itself.

Currently llvm crashes on following code, run through llc:

declare void @llvm.experimental.stackmap(i64, i32, ...)

define void @foo() {
tail call void (i64, i32, ...)* @llvm.experimental.stackmap(i64 0,
i32 0, i64 9223372036854775807)
ret void
}

The issue is that 9223372036854775807 (decimal for 0x7fffffffffffffff)
is the "empty key" for an int64_t, and in
StackMaps::recordStackMapOpers we crash when we try to insert this as
a key into ConstPool. The this happens if we change the constant to
be the tombstone.

Two potential fixes I can think of:

1. have some special logic to remember the offsets for the tombstone
   and empty i64 constants. This can easily be tracked using two
   "Optional<int>" fields.

2. change ConstantPool to be use a std::map instead of a
   llvm::DenseMap as the map in the MapVector.

An aside: the same function has this check "((I->Offset +
(int64_t(1)<<31)) >> 32)" -- won't this cause a signed overflow (and
hence UB) if I->Offset is negative?

I guess "!isInt<32>(I->Offset)” might be a more readable solution.

We also could make the DenseMap just an uint64_t. In this case the empty key and the tombstone would be never encoded into the constant pool, because they can be encoded in the offset field itself.

That's a great idea, I'll send in a patch soon.

-- Sanjoy