HomePhabricator

[mlir] Fix argument attribute attribute reassignment in ConvertStandardToLLVM

Authored by ftynse on Fri, Feb 14, 1:18 AM.

Description

[mlir] Fix argument attribute attribute reassignment in ConvertStandardToLLVM

The commit switching the calling convention for memrefs (5a1778057)
inadvertently introduced a bug in the function argument attribute conversion:
due to incorrect indexing of function arguments it was not assigning the
attributes to the arguments beyond those generated from the first original
argument. This was not caught in the commit since the test suite does have a
test for converting multi-argument functions with argument attributes. Fix the
bug and add relevant tests.

Details

Committed
ftynseFri, Feb 14, 1:22 AM
Parents
rG189c701332eb: [lldb] Remove accidentally checked-in debugging code
Branches
Unknown
Tags
Unknown

Event Timeline

bondhugula added inline comments.
/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
923

Not strictly related to this patch, but this can be hoisted out. (Could otherwise lead to more repeated malloc's for large names.)

/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir
13

Nit: -> Make sure argument attributes are correctly propagated.

15

Shouldn't this be CHECK-SAME? Not sure to me how the tests are passing!

ftynse marked 2 inline comments as done.Fri, Feb 14, 4:05 AM
ftynse added inline comments.
/mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
923

Practically, the attribute name is arg# so there will be no malloc at all unless you have more than 100000 function arguments. I expect you'd have other memory issues to worry about in that case.

/mlir/test/Conversion/StandardToLLVM/convert-argattrs.mlir
15

The matching is done by character, not line. Successive CHECKs do not have to be on different lines, they can be on the same line. CHECK-SAME _requires_ them to be on the same line. I required 'SAME" for the second part to make sure we stay within the argument list rather than accidentally matching something later (e.g., a different function). It would still be fine to drop SAME everywhere.