Wltrcrash fix for Issue #2280.

This is a fix for the WLT crashes described in Issue #2280 (closed). The actual fix involves running a module-level WLUR in phase WLT.

I think the above failure may belong to our friends in emru.

Yes it is coming from EMR! Regarding the failing test, the error message is:

./test-ipbb-faux.sac 24 error: SSATid: Variable _emrl_21042_emr_lifted used without definition

I suspect its because of the lifted avis, which does not at this point have a proper vardec... I would need to look closer to confirm.

A quick fix would be to disable EMR for the ipbb test (e.g. -noEMRCI).

MacOS tests pass because ISL/Barvinok are missing (I think...).

The EMR enhancements appear to also have broken all of the Livermore Loops, here:

https://github.com/SacBase/LivermoreLoops

If I find some time when I get back from the St. Lawrence Market today, I'll take a look at the EMR problem. I think it is odd that the failures didn't get picked up by the merge tests.

A few questions/ remarks:

Why do you distinguish between WLUnrollingModule and WLUnrollingFundef now? Couldn't the original version be applied in both situations?

Also, I would think we should only unroll the minimum needed, which, I assume is 1? We could have another entry function that sets the maxwlurunroll parameter to 1, then calls the generic one, and afterwards sets the parameter back to its original value....

Re WLUR: The original code was called only in CYC and SAACYC, so did not support the N_module call which is needed in WLT. At first, I had the N_module vs. N_fundef decision being made in the WLUR entry function, but using Module/Fundef entries made the code straightforward in both paths.

If you want to mess with the maxwlunroll parameter, my approach should make that quite straightforward. Is there some reason to prefer your single-entry approach?

As for the maxwlunroll parameter: I have no objection to such a change. My own preference would be to introduce a one-trip WL optimization. That would highlight the limited intent of the code change, and perhaps simplify WLUR (which does two jobs today), but I consider it a frill, and would rather fry bigger fish.

I took a quick look at EMR yesterday. the offending value error is in the Loop-fun, in which it does has a vardec, and appears as the last parameter in the recursive call, but it has no assign (obviously).

I don't see any code in the EMR*.c code that invokes TBmakeAssign, which would explain why we get those value errors. I think the N_avis might be fine, but a missing assign would fix it, if we can find out where it goes and what its value should be! Back to you, Hans.

added 1 commit

• f5267ed9 - [emr] create assign for EMRL for WLTR fix

Compare with previous version

Hi Bob, I've created a fix for the EMRL traversal... in hindsight it was silly of me to not create an assignment to the newly minted EMR lifted N_avis. Current implementation uses a noop() for the assignment until we hit mem:alloc, at which point we update this to an alloc.

This should resolve the crash in SSATransform, and now the LivermoreLoops compile completely.

Thanks, Hans,

I give all of those a try now. I'll amend this report after that.

I am curious about the missing assignment: how did that get through the gitlab merge?

added 1 commit

• 98fa354c - remove comments

Compare with previous version

added 1 commit

• 3bff20f9 - remove comments

Compare with previous version

The Livermore Loops now build correctly, but ctest is absent, so I have not tested the generated code.

Hi Bob, if there's nothing more you want to add to this, can we merge it in?

Please do. Thanks, Bob

merged

mentioned in commit 8fa2b382

mentioned in issue #2280 (closed)

mentioned in issue #2297 (closed)