This is an archive of the discontinued LLVM Phabricator instance.

Initialize IsFast* values
ClosedPublic

Authored by bcain on Mar 13 2020, 1:27 PM.

Details

Summary

We must initialize these values in case some targets do not assign to them in allowsMemoryAccess().

Diff Detail

Event Timeline

bcain created this revision.Mar 13 2020, 1:27 PM
RKSimon accepted this revision.Mar 13 2020, 3:17 PM

LGTM

This revision is now accepted and ready to land.Mar 13 2020, 3:17 PM
bcain added a comment.Mar 13 2020, 4:01 PM

This change seemed inocent enough. Were the false values here the right defaults?

Could this regression really be due to this change?

http://lab.llvm.org:8011/builders/openmp-gcc-x86_64-linux-debian/builds/5090/steps/test-openmp/logs/FAIL%3A%20libomp%3A%3Abug_nested_proxy_task.c

This change seemed inocent enough. Were the false values here the right defaults?

Yes, we tend to check for IsFast=true after most successful allowsMemoryAccess calls so false is the better default. But really if a target allowsMemoryAccess implementation is going to return true it should always explicitly set the IsFast value as well.

Could this regression really be due to this change?

http://lab.llvm.org:8011/builders/openmp-gcc-x86_64-linux-debian/builds/5090/steps/test-openmp/logs/FAIL%3A%20libomp%3A%3Abug_nested_proxy_task.c

The openmp buildbots are notoriously unreliable - my recommendation for those bots is always to wait for another build to see if the red just goes away.....