[IR][Verifier] Allow IntToPtrInst to be !dereferenceable

[rtaylor]

Summary

Allow IntToPtrInst to carry !dereferenceable metadata tag.
This is valid since !dereferenceable can be only be applied to
pointer type values.

Change-Id: If8a6e3c616f073d51eaff52ab74535c29ed497b4

[arsenm]

test/Verifier/dereferenceable-md.ll

88–90

Can you also add a test for defeferencable_or_null

[jdoerfert]

test/Verifier/dereferenceable-md.ll

91

And all other tests as well, e.g., a positive test (not here), wrong type, wrong number of operands, etc.

[arsenm]

Also needs a langref update

[rtaylor]

Added tests and updated LangRef

[arsenm]

test/Verifier/dereferenceable-md-inttoptr.ll

1–4 ↗

(On Diff #210900)

Verifier tests should not run llc and cannot depend on a target. Most verifier just uses llvm-as

[rtaylor]

Added !dereferenceable_or_null to syntax of inttoptr in LangRef

[rtaylor]

Fixed test to use llvm-as and to just check for passing.

[jdoerfert]

Are you planning to teach Value::getPointerDereferenceableBytes about this? The rest of the pipeline "should" then "just work" and use the information.

docs/LangRef.rst

9662 ↗

(On Diff #210911)

Copy & Paste, these are no loads. Remove last sentence and add load metadata to the list of analogous things.

lib/IR/Verifier.cpp

3986

Why ^? I personally find that confusing. || should do just fine.

[rtaylor]

Changed LangRef.rst to reflect !dereferenceable is applicable to pointer types not
just load types.

Changed logic in verifier.

[rtaylor]

Added IntToPtrInst to case statement in Value::getPointerDereferenceableBytes

[jdoerfert]

I'm fine with this, let's give @arsenm a chance to look at it as well.

[arsenm]

LGTM

[nhaehnle]

Thanks! Could you please also add a test to Analysis/ValueTracking/memory-dereferenceable.ll?

docs/LangRef.rst

8622 ↗

(On Diff #210970)

Propose a change of the wording here to: This metadata can only be applied to certain instructions producing a pointer value.

After all, the metadata is not applied to the type itself. Same goes in the next paragraph.

[rtaylor]

docs/LangRef.rst

8622 ↗

(On Diff #210970)

Are there certain instructions that produce a pointer value in which !dereferenceable cannot be applied? If so, then maybe we should list the instructions in the description(s) that it can be applied, for clarity? I'd rather not leave this as vague as "certain instructions" because the reader's first thoughts are going to be "which instructions?" Certainly if the description exists for every instruction than one could deduce it by why be vague if we don't have to?

[rtaylor]

docs/LangRef.rst

8622 ↗

(On Diff #210970)

Also, the current Parameter Attributes reads quite simliar: This attribute may only be applied to pointer typed parameters.

So in this case, it's actually attached to the type and not the instruction?

[arsenm]

docs/LangRef.rst

8622 ↗

(On Diff #210970)

Calls aren't allowed because they have matching attributes that are superior to the metadata. Other cases I think are less clear and may make sense to allow it in the future.

I think this section should be split out. As is, you are just updating the notes on metadata allowed for loads. The load and inttoptr can then reference the !dereferencable section

[rtaylor]

docs/LangRef.rst

8622 ↗

(On Diff #210970)

Agree. I think these should probably have their own '`....`' Metadata section?

[rtaylor]

Changed LangRef.rst: Moved !dereferenceable and !dereferenceable_or_null to their
own section in Metadata and pointed inttoptr and load to that section.

Added test for Analysis/ValueTracking.

[nhaehnle]

docs/LangRef.rst

5306–5324 ↗

(On Diff #211112)

Could you add the incantations for hyperlinks? ".. _blahblah" for the anchor and :ref:.... <blahblah> for the link.

[rtaylor]

Added hyperlinks to LangRef.rst

[nhaehnle]

LGTM, though I believe the hyperlinks in :ref: need to be without the underscore (and other examples of :ref: use seem to corroborate that).

[rtaylor]

Removed preceding underline to ref hyperlink.

[rtaylor]

Remove underline to ref hyperlink from invariant.group

lib/IR/Verifier.cpp

void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
   const Use &U = I.getOperandUse(i);
   Assert(DT.dominates(Op, U),
          "Instruction does not dominate all uses!", Op, &I);
 }
 
 void Verifier::visitDereferenceableMetadata(Instruction& I, MDNode* MD) {
   Assert(I.getType()->isPointerTy(), "dereferenceable, dereferenceable_or_null "
          "apply only to pointer types", &I);
-  Assert(isa<LoadInst>(I),
+  Assert((isa<LoadInst>(I) || isa<IntToPtrInst>(I)),
          "dereferenceable, dereferenceable_or_null apply only to load"
          " instructions, use attributes for calls or invokes", &I);
   Assert(MD->getNumOperands() == 1, "dereferenceable, dereferenceable_or_null "
          "take one operand!", &I);
   ConstantInt *CI = mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
   Assert(CI && CI->getType()->isIntegerTy(64), "dereferenceable, "
          "dereferenceable_or_null metadata value must be an i64!", &I);
 }

test/Verifier/dereferenceable-md.ll

 
 
 define i8* @f10(i8** %x) {
 entry:
   %y = load i8*, i8** %x, !dereferenceable_or_null !{i32 2}
   ret i8* %y
 }
 ; CHECK: dereferenceable, dereferenceable_or_null metadata value must be an i64!
-; CHECK-NEXT: load i8*, i8** %x
+; CHECK-NEXT: load i8*, i8** %x
+
+define i8* @f_0(i64 %val) {
+  %ptr = inttoptr i64 %val to i8*, !dereferenceable !{!"str"}
+  ret i8* %ptr
+}
+; CHECK-NOT: dereferenceable, dereferenceable_or_null apply only to load instructions, use attributes for calls or invokes