[HIPIFY][#837][#2062][#2415] Add multi-declaration gaurd check for e_insert_new_argument - Part 2

[ranapratap55]

Context

Follow-up PR to #2415 for e_insert_new_argument cast type that generates variable declarations above a function call when hipifying APIs where MIOpen requires additional arguments not present in CUDA.

One known limitation in #2415 was:

Multiple-declaration guard: If the same API is called twice in the same scope,
the hipify_ variables would be declared twice, causing a compile-time redefinition
error.

With this PR, we are fixing it.

Problem

Given a CUDA code with the same API called multiple times in the same scope:

status = cudnnSetDropoutDescriptor(desc, handle, dropout, states, size, seed);
status = cudnnSetDropoutDescriptor(desc, handle, dropout, states, size, seed);

#2415 would generate

bool hipify_use_mask = {};       // works
bool hipify_use_mask = {};       // fails — REDEFINITION ERROR

Solution

Added InsertedVarCounter that tracks the count for each base variable name that has been generated.

Also fixed warning emission loop with insertionVarNames vector so the compiler warning message correctly reports the unique name hipify_use_mask_1 instead of the base name hipify_use_mask.

With this change code would generate like:

bool hipify_use_mask = {};         // works
bool hipify_use_mask_1 = {};       // works - unique name
bool hipify_use_mask_2 = {};       // works - unique name

[emankov]

FileID is still needed to prevent loss of translation-unit isolation.
What about using std::tuple inside the map?

[ranapratap55]

Done. Changed the map type to std::map<std::tuple<clang::FileID, unsigned, std::string>, unsigned>.

[emankov]

Excessive check?

[ranapratap55]

Fixed.

[emankov]

[Assumption] The if-else construct without curly braces in the source CUDA code will lead to syntactically incorrect HIP code, for instance:

if (condition)
        status = cudnnSetDropoutDescriptor(desc, handle, dropout, states, size, seed);
    else
        status = CUDNN_STATUS_SUCCESS;

[ToDo] The if-else construct without curly braces should be presented in tessts.

[ranapratap55]

yes, thanks. This is a ToDo 1 from #2415, Added a // TODO: block in .cpp and test file.

[emankov]

AST traverse here should cover all parents, not only the first one, which are very common in template Instantiations, macros, and implicit conversions.

[ranapratap55]

Thanks for this. Fixed in the latest revision.
Corrected to parent.get<clang::CompoundStmt>(), so every node at the current AST level is checked before ascending.

[emankov]

[Performance] The expensive AST tree climb to find the CompoundStmt three separate times for the exact same function call.

[ranapratap55]

Thanks and fixed.
Moved the logic outside the for (auto c : cc.castMap) loop and AST climb happens exactly once.