@sdmaclea - I'm not certain this is the right fix, but I added an assert that the type of op2 matches the type of the LOCKADD before it modifies that to TYP_VOID. It eliminates the assert in crossgen'ing of SPC.dll. I'll wait to hear from you whether there are tests for which the assert in gentree.cpp will fail.

Something between gentree and codegen is changing the type of the addend

On my tip w/o your original change, I added three asserts, that the addend matched the atomic node type.

1. Importer (creation of atomic HIR)
2. gentree (your assert from this patch)
3. codegen (since I still have the TYP_VOID assignment #ifdef out)

This test is hitting the codegen assert on my tip.
JIT/Regression/JitBlue/GitHub_10714/GitHub_10714/GitHub_10714.sh

gdb from failing test case JIT/Regression/JitBlue/GitHub_10714/GitHub_10714/GitHub_10714.sh:

Assert failure(PID 44402 [0x0000ad72], Thread: 44402 [0xad72]): Assertion failed 'emitTypeSize(treeNode) == emitTypeSize(data->gtType)' in 'GitHub_10714:Test():int' (IL size 16)

    File: /home/vmjenkins/workspace/Dotnet/build_and_test/src/jit/codegenarm64.cpp Line: 2732
    Image: /home/sdmaclea/coreoverlay/corerun


Thread 1 "corerun" received signal SIGABRT, Aborted.
0x0000ffffb7bb1528 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
54      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) frame 9
#9  0x0000ffffb31d11b0 in CodeGen::genLockedInstructions (this=0x4d6ce0, treeNode=0x4d9da8) at /home/vmjenkins/workspace/Dotnet/build_and_test/src/jit/codegenarm64.cpp:2732
(gdb) print data->gtType
$1 = var_types::TYP_SHORT
(gdb) print treeNode->gtType
$2 = var_types::TYP_INT
(gdb) print data->isContainedIntOrIImmed()
$3 = false
(gdb) print data->OperGet()
$4 = genTreeOps::GT_IND

So the addend is from a indirect TYP_SHORT address. During lowering, there must have been a cast dropped. Since on ARM64 the type of op2 would normally be evaluated with emitActualTypeSize by consumers.

I think GTF_NOVALUE, might be a better way to signal this behavior.

If we want to continue setting GT_LOCKADD->gtType to TYP_VOID, then using emitActualTypSize instead of emitTypSize would solve the problems with atomics here.

I think GTF_NOVALUE, might be a better way to signal this behavior.

I would like to avoid the use of GTF_NOVALUE. In fact, I hope to eventually elimiante it.

If we want to continue setting GT_LOCKADD->gtType to TYP_VOID, then using emitActualTypSize instead of emitTypSize would solve the problems with atomics here.

I think that's the right approach.

@sdmaclea - Do you agree that this is an appropriate and correct fix?
@dotnet/jit-contrib PTAL

@dotnet/jit-contrib - I meant to ping on this, not on the bug.

@CarolEidt Thanks, I had to go offline last night due to a household appliance failure

@CarolEidt BTW, there may still be an issue for 64 bit atomics if the addend is an indirect small type, when there should be a carry into the high 32 bits. I would guess the problem exist on all platforms now, but I am not sure. There is certainly no coverage for this case.

there may still be an issue for 64 bit atomics if the addend is an indirect small type, when there should be a carry into the high 32 bits. I would guess the problem exist on all platforms now, but I am not sure. There is certainly no coverage for this case.

Agreed. I opened #14557 because I think the implementation of the interlocked operations would be cleaner and less error-prone if they took an indirection as their op1. That indirection would hold the correct type. I'll add a note there that we should add better test coverage.