This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Added llvm.freeze
ClosedPublic

Authored by Sagar on Feb 27 2020, 10:51 PM.

Details

Summary

This patch adds llvm.freeze & processes undef constants from LLVM IR.

syntax:
LLVM IR
<result> = freeze ty <val>

MLIR LLVM Dialect
llvm.freeze val attr-dict : type

Example:
LLVM IR: %3 = freeze i32 5
MLIR: %6 = llvm.freeze %5 : !llvm.i32

Diff Detail

Event Timeline

Sagar created this revision.Feb 27 2020, 10:51 PM

Can you document the semantics somewhere? (I have no idea if other patches have done anything like this, but it does seem like something that should be documented somewhere).

Sagar updated this revision to Diff 247178.Feb 28 2020, 12:57 AM

Had not run clang-format.

Looks good, I mostly have nits.

Can you document the semantics somewhere? (I have no idea if other patches have done anything like this, but it does seem like something that should be documented somewhere).

In general, we should just refer to the relevant piece of LLVM's LangRef unless MLIR modeling has different semantics than LLVM IR (which we should avoid as much as possible).

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
511

Nit: could you keep these ordered in the same way as LLVM's LangRef?

mlir/test/Dialect/LLVMIR/roundtrip.mlir
288

Please avoid matching SSA names (%arg0). You can capture by putting // CHECK-SAME: %[[ARG0:.*]]: after the label

290

Nit: we usually just do %[[x:.*]]

ftynse requested changes to this revision.Feb 28 2020, 1:16 AM
This revision now requires changes to proceed.Feb 28 2020, 1:16 AM
Sagar updated this revision to Diff 247232.Feb 28 2020, 5:09 AM

NIT changes done.

ftynse accepted this revision.Feb 28 2020, 6:18 AM

Thanks!

This revision is now accepted and ready to land.Feb 28 2020, 6:18 AM
Sagar marked an inline comment as done.Feb 28 2020, 7:33 AM
This comment was removed by Sagar.
Sagar added a comment.Feb 28 2020, 7:34 AM

I do not have write access to the repository. If would be great if someone could commit for me.

Which name and email should we associate with the commit?

Sagar added a comment.EditedFeb 28 2020, 9:18 AM

This is my Github account, https://github.com/Sagox
primary email address: cs17btech11034@iith.ac.in
for name you could use my Github username or the one on Phabricator, doesn't really matter.

This revision was automatically updated to reflect the committed changes.