This is an archive of the discontinued LLVM Phabricator instance.

[mlir] [VectorOps] Add create mask integration tests
ClosedPublic

Authored by aartbik on Jun 16 2020, 12:20 PM.

Details

Summary

Two integration tests focused on i1 vectors, which exposed omissions
in the llvm backend which have since then been fixed. Note that this also
exposed an inaccuracy for print_i1 which has been fixed in this CL:
for a pure C ABI, int should be used rather than bool.

Diff Detail

Event Timeline

aartbik created this revision.Jun 16 2020, 12:20 PM
aartbik edited the summary of this revision. (Show Details)
bkramer accepted this revision.Jun 17 2020, 9:49 AM

I'm a bit worried that we now call a function taking an int with an i1 argument. This probably works because the x86 ABI passes it in a big register anyways, but it's technically undefined behavior. Can we make this int32 and call it with i32? This is fine for a followup.

This revision is now accepted and ready to land.Jun 17 2020, 9:49 AM

I'm a bit worried that we now call a function taking an int with an i1 argument. This probably works because the x86 ABI passes it in a big register anyways, but it's technically undefined behavior. Can we make this int32 and call it with i32? This is fine for a followup.

It was my understanding that calling from C++ ABI with i1, i.e. bool, to C ABI using int is correct. But if you are worried we can indeed fix this in a follow up by letting the vector.print lowering part force it into a int32 and use that instead as parameter.

This revision was automatically updated to reflect the committed changes.